Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: 64bit nanosecond jmapids #5177

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

ksmurchison
Copy link
Contributor

@ksmurchison ksmurchison commented Dec 19, 2024

TODO:

  • Add coded for new mailbox ids (either conv.db folder num or createdmodseq)
  • Write a test for new code running with v19 mailboxes and v1 conv.db

@ksmurchison ksmurchison marked this pull request as draft December 19, 2024 15:21
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch from be60440 to 5599d95 Compare December 20, 2024 18:43
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch 2 times, most recently from 63fc02b to 4fb40bd Compare January 6, 2025 14:15
@ksmurchison ksmurchison requested a review from rsto January 6, 2025 14:16
@rsto rsto removed their request for review January 8, 2025 08:00
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch 5 times, most recently from 5d8190f to 2e5b103 Compare January 15, 2025 19:12
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch 9 times, most recently from 33bef57 to deb2012 Compare January 22, 2025 17:06
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch from deb2012 to 8141e3d Compare January 23, 2025 19:44
@ksmurchison ksmurchison requested review from brong, rsto and elliefm January 26, 2025 20:16
@ksmurchison
Copy link
Contributor Author

ksmurchison commented Jan 26, 2025

This isn't complete yet but could probably use a sanity check. Reviewing commit-by-commit would probably be the easiest.

@rsto
Copy link
Member

rsto commented Jan 27, 2025

@ksmurchison since this is a WIP review, could you please let me know the main file or functional areas you would like me to sanity check?

@ksmurchison
Copy link
Contributor Author

@ksmurchison since this is a WIP review, could you please let me know the main file or functional areas you would like me to sanity check?

You should probably look at conversations.c and jmap_mail.c

@rsto
Copy link
Member

rsto commented Jan 27, 2025

You should probably look at conversations.c and jmap_mail.c

Thanks, will do.

Copy link
Member

@rsto rsto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave conversations.c and jmap_mail.c a read. It looks to implement the requirements, I just have a question about refcounts as well as some thoughts about function names.

@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch from 694df67 to b57c6b2 Compare January 27, 2025 13:41
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch 5 times, most recently from 9a09cd0 to ba893ee Compare February 11, 2025 21:33
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch 6 times, most recently from c66f32a to b671245 Compare February 20, 2025 19:04
store the encoded nanosecond value as a J(MAPID) key
that maps to the GUID of an email
use a "virtual" annotation to pass this value via replication
   - use the internaldate of an existing message with same UID
   - pick internaldate.nsec in a deterministic manner
   - make sure internaldate doesn't conflict with an existing JMAPID
   - set the timestamps on the message data file to internaldate
   - update G record and add J record in conv.db
triggered by conv.db version

NOTE: this still needs a test to make sure v1 still works
@ksmurchison ksmurchison force-pushed the 64bit-nanosecond-jmapids branch from b671245 to 368b4da Compare February 21, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants