901419 - [SMS][MMS] Make Database Journalising when Deleting Messages and Threads

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Gene Lian (I already quit Mozilla)]

Please see the item #3 at bug 893838, comment #34.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Why do we need this?

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #1)
> Why do we need this?

To resolve corner cases we currently have where a user might select all messages and then ask for deletion. The operation takes a long time and the user sends the app to the background and it gets eventually killed by an OOM condition; the next time the user will open the app he'll find leftover messages that haven't been deleted yet. We have no way of solving this issue at the app level without having appropriate methods for certain operations (bug 895734) and making those methods effectively atomic from the application perspective (this bug).

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Gabriele Svelto [:gsvelto] from comment #2)
> To resolve corner cases we currently have where a user might select all
> messages and then ask for deletion. The operation takes a long time and the
> user sends the app to the background and it gets eventually killed by an OOM
> condition;

Doesn't sound like a issue in SMS.  Every app can suffer from this.

> the next time the user will open the app he'll find leftover
> messages that haven't been deleted yet.

That's definitely an issue.  I thought we will continue to delete messages even child process is dead.  And since this is a write transaction, other read transactions should be blocked.  So we should not have any message undeleted, am I wrong?

> We have no way of solving this issue
> at the app level without having appropriate methods for certain operations
> (bug 895734) and making those methods effectively atomic from the
> application perspective (this bug).

bug 895734 won't change anything here.  Gecko still has to delete thousands of messages and gives no response back during the process.

Comment 4

[Julien Wajsberg [:julienw]]

Yep maybe there is really nothing to do Gecko side.

We probably need to gather in a room and write stuff on a white board during the workweek once Gabriele will be there, with Borja and Gene too.

Comment 5

[Rick Waldron [:rwaldron]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #3)
> (In reply to Gabriele Svelto [:gsvelto] from comment #2)
> > To resolve corner cases we currently have where a user might select all
> > messages and then ask for deletion. The operation takes a long time and the
> > user sends the app to the background and it gets eventually killed by an OOM
> > condition;
> 
> Doesn't sound like a issue in SMS.  Every app can suffer from this.
> 
> > the next time the user will open the app he'll find leftover
> > messages that haven't been deleted yet.
> 
> That's definitely an issue.  I thought we will continue to delete messages
> even child process is dead.  And since this is a write transaction, other
> read transactions should be blocked.  So we should not have any message
> undeleted, am I wrong?

If I "Delete All" and the app is closed, then re-openned, it's very reproducable to see threads with messages re-appear, despite having been deleted.

Comment 6

[Julien Wajsberg [:julienw]]

Rick, are they eventually deleted at one point, if you kill the app again and load it again later ?

Comment 7

[Gene Lian (I already quit Mozilla)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #3)
> > the next time the user will open the app he'll find leftover
> > messages that haven't been deleted yet.
> 
> That's definitely an issue.  I thought we will continue to delete messages
> even child process is dead.  And since this is a write transaction, other
> read transactions should be blocked.  So we should not have any message
> undeleted, am I wrong?

I had the same feeling when I saw this issue.

I'm doubting it's because the threads and message are maintained by different DB stores. That is, we use getThreads() to retrieve threads from thread DB store which hasn't got updated yet while messages are getting deleted in the message DB store.

Julien is pointing out a good idea at comment #6. We need to check if the messages will get deleted in the long run. If they won't, something must be wrong in the DB itself.

Comment 8

[Caio Lima(:caiolima)]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Rick, are they eventually deleted at one point, if you kill the app again
> and load it again later ?

I'm going to test this situation. BTW, Have you discussed anything on work week?

Comment 9

[Julien Wajsberg [:julienw]]

nope ;)

Comment 10

[Julien Wajsberg [:julienw]]

The initial issue seems to work for me now.

In my test:

* when deleting all messages in a thread, it seems to continue in background. So killing the app doesn't change anything.
* when deleting several threads, we first (1) gather all message ids for these threads and then (2) deleting these ids. Killing the app while (1) will keep all the conversations while killing the app while (2) will delete them all.

This likely changed once "delete" was modified to accept several ids at once.

So it's a wontfix for the journalizing feature. That said the expected mozMobileMessage API v2 is definitely more like a 2-pass commit API so this is quite close to this.