[0.2.1-deb] Deleted source briefly re-appears as ghost

[eloquence]

Encountered during QA.

After deleting a source just before a sync, I saw the source briefly re-appear for a few seconds. The ghost of the source did not have any messages or replies, and communicating with the ghost led to reply failures. The ghost source vanished after a few more seconds.

For cases like this, a pending deletion state (#534) would come in handy.

[sssoleileraaa]

Can you check if you can still reproduce now that #866 has been merged (also I haven't been able to repro this issue yet)?

Update

I just saw this issue

[eloquence]

I've not tested with #866 yet but I saw it again in the original 0.2.1-deb release build today.

[sssoleileraaa]

ah good to know, i'll keep trying to repro today

[sssoleileraaa]

I just want to add my notes to this issue:

Why does deleting the source from the local db before we see the source gone in the next sync cause the ghost issue?

1. A sync happens right before a source deletion on the server and the server response says the source exists
2. Deletion of source finishes on the server and the source is removed from the local db.
3. The sync success handler will notice that we don't have that source in our local db, so it will think it's a new source and add it
4. The next sync gets a server response without the source (it no longer exists)
5. The sync success handler will notice that we have the source in our local db, so it will delete it.

This is an issue because we assume in the client code that a source from a server sync response (which could be outdated) that we don't have stored in our local db is a new source that we need to add locally.

If instead of deleting the source locally we just updated its status to DELETED we would know that the source we get back from the server is just from an outdated response.

When should we remove the source from the local sources table?

1. We could remove the DELETED source once we get a sync response with the source gone from the list we get back from the server. If there is a crash, then the next time the user logs in the sync will happen and it will remove any DELETED sources if the response shows them as missing

2. We could remove any DELETED sources on logout or login (if there was a crash).

[redshiftzero]

Agree with your 1. So recapping a convo between @creviera @eloquence @ninavizz and I, we can:

1. When we click source delete, we immediately set the source to its pending state (per @ninavizz mockup in Need a pending source deletion state #534).
2. When source deletion completes, we transition the source status from pending to deleted per your comment. The source deletion success callback deletes the source's files, messages, replies. At this point we'd no longer show the source in the source list (i.e. we need to filter by sources that are not marked as deleted).
3. The sync logic will only add new content for sources that are marked as not deleted locally (i.e. we should not add download jobs for their messages/replies). The source row will be deleted by this sync action.

And we could separate this into two changes:

1. Implement backend changes (resolves this item)
2. Display pending status in UI when source is in pending state (resolves Need a pending source deletion state #534)

[sssoleileraaa]

And we could separate this into two changes:

1. Implement backend changes (resolves this item)
2. Display pending status in UI when source is in pending state (resolves Need a pending source deletion state #534)

Agreed, just want to note (already shared off-github with @redshiftzero) that we can achieve the above without a DELETE_PENDING db state. We only need to update the db once when the source is DELETED instead of twice: when it's deleted and when the user requests to delete the source (DELETE_PENDING). Unlike pending replies, we don't need to show sources permanently as "failed" so we don't need this pending state in the db: pending would only show up in the gui and last until we get a success or failure signal from the job. If it succeeds we would set the source as DELETED and the remove the source (in a pending state) from the GUI. If it fails we would show an error message and the user will have to manually try again. If the session ends/token expires, the user would lose the queued up jobs anyway, so the user will have to manually re-delete anyway.

[sssoleileraaa]

To explain this another way, there's no reason we would ever need to check the db to see if we need if we need to show a source as pending deletion before creating a source widget.

Also, removing the DELETE_PENDING db state means we don't have to check for it on login or logout to set it to FAILED or None or something. A source either exists, doesn't exist, or is in this DELETED state to fix the ghost issue.

[eloquence]

I follow this reasoning; in that model, a DELETED source would just be a placeholder entry in the DB (to ensure thread safety with any unfinished sync operations) with no messages or replies, that is finally fully removed as part of startup/logout cleanup, correct?

[redshiftzero]

Following up on our earlier conversation, I was looking at the SourceList to see if any problems would occur from storing the pending state on an individual SourceWidget. We are currently clearing and recreating the source widgets when we add a new source. If you trace the execution of this function it recreates each SourceWidget each time the list gets redrawn. So we should either not store state on that object or modify that behavior.

[sssoleileraaa]

So we should either not store state on that object or modify that behavior.

#893 modifies that behavior. Once it's merged we will only delete SourceWidget when the source is deleted, which resolves the issue you pointed out.

[redshiftzero]

I agree - since we do not need to persist the pending status between sessions (the journalist will need to attempt to delete the source again manually in that scenario), we do not need to store it in the database after #893