Comment 36 for bug 2060534

Revision history for this message
In , Benc-q (benc-q) wrote :

(In reply to omry from comment #34)
> No, the problem here is not related to compacting a folder after deleting all the messages.
> Good to hear you rewrote that code. Can you point to your diff?

See Bug 1890448. There's a patch in phabricator attached there.

> A few points/ideas that would make things more robust (some mentioned above):
> 1. Diligent error handling while writing the tmp file.

Indeed.

> 2. Making sure that multiple processes/threads would not write to the same tmp file concurrently.

There is some folder locking, but nothing at the FS level. All folder stuff is on the main thread anyway (so the JS frontend can use it).

> 3. Always cleaning up compaction tmp files on error.

Yup. Using an existing file class which deletes itself if an explicit commit is not issued.

> 4. Clean up compaction tmp files on startup on case of an unclean shutdown during compaction.

Nope... but that's mostly covered by 3. A hard power-down or extreme crash would probably leave something behind, I think...
Potentially could add a startup check, but there are some corner-case issues there too.

> 5. Ensure that the number of written messages while compacting does not exceed the number of messages in the folder being compacted.

Not a problem with the new code. The store iterates through the existing messages as it writes out the new mbox, and never revisits ones it's already written or skipped.