Bug: reordering the jam on multiple phones at the same time causes the jams to be out of sync

[JayThomason]

Attempting to reorder the jam on both client phones at the same time can cause the jam to become out of sync on the client phones.

When I tested this, only one of the client phones was out of sync while the other was in sync with the master. I think that this is due, at least in part, to two things: 1) insufficient information being transferred from the master to the client phones 2) the client phone accepting the jam changes made by the user before accepting the jam changes made by the master. Messages from the master about reordering the jam should take precedence over user actions, even if this means the jam sort of jumps around after updates from the master come to the clients.

[hwray]

I've thought about this quite a bit, and I don't necessarily see an easy solution. We could have 3/4 or more clients all spamming the master phone with multiple drag and drop requests per second, and we have no way of ensuring that they are all received by the master or by the other clients in the correct order.

The only high-level solution I see is to move away from sending hyper-specific requests such as "move song X to index Y", "set song X to be currently playing", etc. Instead, we should consider the advantages of just having the master rebroadcast the entire state of the Jam to all Clients after certain operations. We would also have to timestamp these broadcasts to ensure the Clients don't receive an old update that has been delayed, and desync from the actual current state.

[JayThomason]

I don't think there is an easy solution either. It also seems to me that the thing to do would be to have the master rebroadcast the state of the jam with every change made to the jam.

[hwray]

That may be the only way to go, although it will result in some client operations having unintended effects For example, if one client clicks to play a song just after another client sends a message to the master to swap that song to another position in the Jam. The master will end up playing the new song that it already thinks is swapped to that position, rather than the song the client actually clicked on

[hwray]

We could maybe side-step some of these issues by using heuristics. For example, for /jam/setSong requests, send the name of the song along with its index in the jam. If a concurrent change has been made and the song at that index of the Jam no longer has the same name, don't go through with the /setSong operation.

Damn, things are getting complicated. :-1:

[JayThomason]

Is there a way that we can just send the unique id of the song? I don't remember our status on that or if it still existed in the database.

[hwray]

It does exist in the database, but the same song can exist in the jam multiple times :/

We could start generating new unique IDs for songs in the Jam though.

[hwray]

That is actually definitely the move. We should just append an iterated number to the end of the existing song hash code when adding that field to the Jam database.

[JayThomason]

Oh shoot. Wasn't even thinking of that. Yeah we'll have to think about this a bit and come up with a smart way to handle this.

[hwray]

Even after implementing the unique ID thing, the master Jam is still freaking out when both clients rearrange songs simultaneously. No crashes, but songs on the master start reordering themselves and changing text randomly (clients seem to stay consistent with each other tho, which is weird).

I think what's happening is that the two move requests end up modifying the master jam's database simultaneously. We may have to come up with some kind of global lock to prevent concurrent calls to move methods in the Jam.

[JayThomason]

Theoretically it shouldn't be possible for the database to be modified by more than one thread at once since our DatabaseHandler is a singleton and apparently the class it extends just serializes all reads/writes over a single connection to the db (see: https://github.com/JayThomason/Tutti/issues/207).

We might want to just take a closer look at the ordering of events on the master phone while debugging this issue, or we might just go ahead and implement the jam lock seen in https://github.com/JayThomason/Tutti/issues/207.

[hwray]

Seeking clarification on this. Does the synchronization/serialization occur at the method level in the DatabaseHandler, or just on the operation level (individual db.update calls)?

If the former is the case, great. If the latter is the case, then I think we do need to do some method synchronization in the DBHelper. For example, the drag-and-drop functionality makes several updates: change the dragged song's index to a temp value, reorder all the other songs in the Jam accordingly, then replace the dragged song at the correct final index. Unless the DBHelper methods themselves are serialized, I think we'll still have some issues.

@JayThomason let me know your thoughts on this. I really appreciate your networking and concurrency expertise in helping me think about this.

[JayThomason]

The synchronization occurs on the actual sql statements sent to the database. If we are performing executing multiple sql statements within the same method and the later statements depend on the changes made in the previous statements then we do need to synchronize on the methods.

I'm not as familiar with the DBHandler code as you are but after looking at the code for changeSongIndexInJam it is clear that we either need to synchronize on the methods of DBHandler or on a higher level.

[hwray]

I fixed this by synchronizing the Server methods. See update here:

https://github.com/JayThomason/Tutti/issues/207