Location field blank on reload

[decentninja]

Looks like the default location are blanked out of the location field on first reload. The assistants are really confused.

[adrianblancode]

This isn't something we have caught in our testing, right now the ugly fix is to refresh the page and then update the location and comment afterwards.

Some observations: Chrome shows that we call the API with correct payloads, and they return 200 OK. The bus sends the location and comment to all other users in the queue correctly, so it seems that the API works fine. When I only write the location and queue it works normally, but when I write both location and comment then exactly one of them dissapears at random after refresh every time so it seems to be something serverside.

Emil had a theory that it's a concurrency issue in the database where the calls overwrite the queueposition, this could be worked around with a minor frontend hotfix.

[Emill]

Idag när jag hade chrome-developern öppen hände det här:

1. Anrop för att skapa köplats körs och returnerar 200.
2. Anrop för att sätta comment returnerar 404
3. Anrop för att sätta location returnerar 404

Jag tror problemet är att alla anropas skickas samtidigt på en gång istället för synkront (eller att de inte väntar till den första är klar).
Det innebär att backenden inte har hunnit skapa raden i databasen innan anrop 2 och 3 kommer in, därför returnerar de 404. Direkt syns nu på hemsidan att jag har köat mig, men både location och comment är tomma. Om jag laddar om sidan är de fortfarande tomma.

Det andra vanliga scenariot är att databasraden hinner skapas, men endast en av location och comment sparas korrekt (den andra blir tom). Jag antar att det beror på att det ORM som ni använder för att uppdatera en rad gör följande:

1. Läser in hela raden i minnet från databasen.
2. Ändrar det fält som ska ändras.
3. Sparar tillbaka hela raden till databasen.
   Om detta görs samtidigt blir det fel, eftersom två trådar läser in tomma rader, sedan sätter vardera tråd det fält som ska uppdateras, sedan sparas en halvkomplett rad två gånger, så den andra tråden skriver över vad den förra hade skrivit med en tom kolumn. Tyvärr finns det ORMs som inte bara uppdaterar det som faktiskt har uppdaterats, utan skriver även tillbaks de gamla värdena den läste in...

En quick-and-dirty-solution vore att i javascript göra om så anropen görs synkront (man väntar alltid till det förra har returnerat 200).

Nuvarande kod jag hittade:

                                    queues.joinQueue(queueName, userName);

                                    // HACK: Places a timeout so there is time to join the queue
                                    setTimeout(function () {
                                        if (locationform.$valid) {
                                            queues.changeLocation(queueName, userName, location);
                                        }

                                        if (commentform.$valid) {
                                            queues.changeComment(queueName, userName, comment);
                                        }
                                    }, 500);

är inte synkron. Se till att anropa nästa anrop i callback-eventet för den förra xmlhttprequesten.

En bättre lösning vore att ändra APIt till att i samband med att man joinar en kö så skickas även comment och location med i samma anrop, och när man ändrar comment eller location så måste man skicka med både comment och location. För att göra det extra stabilt borde ni se till att man aldrig får ändra en location eller comment förrän anropet som joinar en i kön har returnerat 200.

En ganska intressant blogpost på liknande topic:
http://www.depesz.com/2012/06/10/why-is-upsert-so-complicated/

[adrianblancode]

@dflemstr Now that the labs have ended for the week, could you please look at this and possibly push a hotfix? The course responsible really want a patch so they can continue using QWait, and I don't feel confident enough to properly implement anything myself without being able to test it first.

[dflemstr]

@Emill we are doing DB transactions correctly so the scenario with the empty rows doesn't happen. What does happen though is the race condition you described.

Also, I don't know where that hack with the timeout came from (and I don't want to find out) but I would like to avoid hacks like that in the future...

Yes, the proper fix is to serialize the calls; I would prefer not to introduce a "create and update" API call I possible. We could of course do that if necessary though.

[Emill]

It is still buggy. Locations are sometimes missing. I also saw one guy today that appeared three times in the same queue.

I think location and comment should be included in the create message. Otherwise it is impossible to guarantee that location will be non-empty. It also seems a waste with three requests instead of one (even though we have 1 Gbps at KTH...)

[adrianblancode]

That is a known edge case, it stems from when the users session has expired and they have become logged out from KTH. In that case the user would be able to join the queue "incorrectly" instead of being rejected for not having a valid session #148

There is a PR up which should fix that