Projekt

Generelt

Profil

Bug #4194

Alvorlig problem med Openseach cache og lokale id'er

Tilføjet af Simon Holt for 22 dage siden. Opdateret for 8 dage siden.

Status:
Needs code review
Prioritet:
Urgent
Tildelt til:
Anslået tid:
URL med eksempel:
Kategorier:
Integration - Brønd - Data og relationer

Beskrivelse

Jf. #3905.

Der er stadig problemer med cache af local ids, som medfører at alle request på lokale id'er fejler og vi altid ender med at bruge basis namespace i statuslisterne.

Problemet er her hvor ikke-cachede id'er tilføjes til request: https://github.com/ding2/ding2/blob/master/modules/opensearch/opensearch.client.inc#L422 

Hvis request oprindelig var baseret på lokale id'er, vil request nu blive lavet med identifier i stedet for localIdentifier, da det er setObjectIds() der bruges og ikke setLocalIds(). Det er fordi identifier er den der først tjekkes i TingObjectRequest og bruges hvis den er sat (derfor har denne fejl ingen betydning for request baseret på identifier/PID).

Løsningen må være også at håndtere lokale id'er separat ligesom det gøres ovenfor, når requests bliver konstrueret.

PR følger.

Historik

#2 Opdateret af Gitte Barlach for 22 dage siden

  • Status ændret fra New til Needs code review
  • Tildelt til ændret fra Simon Holt til Jesper Kristensen
  • Prioritet ændret fra Normal til Urgent

Hej Jesper 
Kan du se på denne?

#3 Opdateret af Simon Holt for 16 dage siden

Har fundet yderligere fejl i caching af request baseret på local id og opdateret PR med rettelser. Har nu testet og verficeret at:

1. På en tom cache laver den et korrekt request på localIdentifier

2. Hvis et identisk getObject request med lokale id'er ligger cachen, rammer den cachen og laver ikke et nyt request.

3. Hvis nogle af objekterne ligger i cachen tager den dem og laver et request på de resterende. De cachede resultat og resultat af request merges korrekt sammen til sidst.

4. I alle tilfælde returneres objekterne korrekt og keyed by PID.

Ifb med mit arbejdet i #3936 opdagede jeg, at et par stykker af de titler jeg testede hvor jeg fik "Error missing title" faktisk ikke var fjernlån, men lokale poster der ikke havde fået det rigtig namespace pga af fejlen her. Hvis en lokal post slås op med basis namespace får man et fejlsvar.

Merge af ovenstående PR løste problemet.

#4 Opdateret af Jesper Kristensen for 16 dage siden

@simon arbejder dette sammen med: https://platform.dandigbib.org/issues/4139

som jeg lige ser det er det samme område vi leger rundt i.

#5 Opdateret af Gitte Barlach for 16 dage siden

  • Status ændret fra Needs code review til Reviewed - Needs info/rework
  • Tildelt til ændret fra Jesper Kristensen til Simon Holt

Da PR i #4139 er godkendt og klar, foreslår jeg at vi merger det først. 

#6 Opdateret af Simon Holt for 16 dage siden

Er enig i vi merger #4139 først og jeg skal nok sikre Jespers rettelser spiller med mine i rebase.

Men tror vi bliver nødt til at få merget denne her sammen med rettelsen jeg er ved at lave i https://platform.dandigbib.org/issues/3936.

Det er meget vanskeligt at få #3936 til at spille uden rettelsen her, da der sker nogle underlige ting cahcing laget og den cacher slet ikke.

Får også hele tiden følgende fejl som giver en white page:

"Fatal error: Unsupported operand types in /var/www/drupalvm/drupal/web/profiles/ding2/modules/opensearch/opensearch.client.inc on line 108"

PR i denne sag retter også ovenstående fejl.

#7 Opdateret af Simon Holt for 15 dage siden

Nå men det viste sig at være et helt andet urelateret problem, der gjorde det vanskeligt og at jeg fik alle de fejl: https://github.com/ding2/ding2/pull/1384/commits/e19de5e7ae722f126a1c85390f4efa583375ea02

Det hele så faktisk ud til at spille ret godt til sidst, så tror ikke det er strengt nødvendigt med rettelsen her.

Meeeeen cachingen af getObject request med lokale id'er fungerer slet ikke, så merges disse rettelser her, burde det give generelt bedre performance på statuslisterne.

Og så er der selvføglelig ""Fatal error: Unsupported operand types"-problemet, som man konstant render i, hvis der bliver lavet et getObject request der ikke finder noget objekter.

#8 Opdateret af Simon Holt for 15 dage siden

@Jepser

Jeg har nu testet PR her med dine rettelser fra #4139 merget og det ser ud til at virke fint sammen. Der var selvfølgelig nogle merge-konflikter, men dem kan jeg bare løse når jeg rebaser efter dit PR er blevet merget.

Jeg har også testet begge vores rettelser sammen med min seneste fra #3936 og det spiller bare! For første gang fik jeg faktisk vist informationer om alle de lån/reseveringer jeg burde.

Alle lånestatus siderne virker også til at køre meget bedre og jeg får ikke en masse fejl spyttet ud.

#9 Opdateret af Simon Holt for 15 dage siden

@Jepser Lige en enkelt kommentar: https://github.com/ding2/ding2/pull/1364/files#r263461849 

 

 

#10 Opdateret af Simon Holt for 8 dage siden

  • Status ændret fra Reviewed - Needs info/rework til Needs code review
  • Tildelt til ændret fra Simon Holt til Gitte Barlach

Vil anbefale at vi får merget disse rettelser ASAP. Den bør patches på som en hotfix eller noget.

For uden de vigtige vigtige rettelser til caching laget indeholder PR også en rettelse til en fatal error som jeg konstant støder på, når jeg sidder og tester. Fejlen udløser en WSOD, så det er bestemt meget alvorligt.

#11 Opdateret af Gitte Barlach for 8 dage siden

  • Tildelt til ændret fra Gitte Barlach til Jesper Kristensen
  • Udgave sat til Release 30 - Bugfixes

Eksporter til Atom PDF