From: Eric Wong <firstname.lastname@example.org> To: "Eric W. Biederman" <email@example.com> Cc: firstname.lastname@example.org Subject: Re: Q: Did you do something to message number recently? Date: Tue, 25 Jun 2019 17:51:32 +0000 Message-ID: <20190625175132.ng7oasz6ft3zbhsm@dcvr> (raw) In-Reply-To: <email@example.com> "Eric W. Biederman" <firstname.lastname@example.org> wrote: > Eric Wong <email@example.com> writes: > > > "Eric W. Biederman" <firstname.lastname@example.org> wrote: > >> The add method computes the number using num_for, which uses > >> Msgmpa::mid_insert. > >> > >> Short of the sequence number for msgmap getting scrambled I don't > >> see how that can go wrong. Sigh. > > > > That seems to be what went wrong with "INSERT OR IGNORE" > > > > cf. https://email@example.com/ > > Is it INSERT OR IGNORE or is it the autoincrement? As I read > the sqlite docs autoincrement is allowed to skip message numbers. > Especially in cases where the insert fails. Changing it to plain INSERT made the autoincrement stop skipping. That's been my experience with other DBs in the past, too; but I haven't used "OR IGNORE" much (and it's been a long time since I've looked at SQL outside of SQLite). > This fix should come with a test case shouldn't it? Such a test > case should be pretty straight forward to write. Insert a message > multiple times and then insert a new one. Did you see my changes to t/msgmap.t and t/v2writable.t? I appended the test to existing dupe test in t/msgmap.t and added the num_highwater check to t/v2writable.t which hit duplicates. I don't think there needs to be a separate test, and the test suite is already slow enough to impact productivity on my laptop. > I don't recall the transaction constraints on sqlite but I am wondering > if instead of INSERT OR IGNORE we should read and the increment > num_highwater in the public-inbox code and then we would not need to > depend upon autoincrement only incrementing once. Even when the sqlite > docs don't promise it will. Yeah, that's probably the right step going forward. I don't think there's forward/backwards-compatibility things to worry about, since the indexing code for mirrors already ignores the autoincrement.
next prev parent reply index Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-24 15:59 Eric W. Biederman 2019-06-24 16:34 ` Eric Wong 2019-06-24 17:33 ` Eric Wong 2019-06-24 22:56 ` Eric W. Biederman 2019-06-24 23:42 ` Eric Wong 2019-06-25 12:01 ` Eric W. Biederman 2019-06-25 17:51 ` Eric Wong [this message] 2019-06-24 23:38 ` [PATCH] msgmap: mid_insert: use plain "INSERT" to detect duplicates Eric Wong 2019-06-25 3:48 ` Eric Wong
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://public-inbox.org/README * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190625175132.ng7oasz6ft3zbhsm@dcvr \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
user/dev discussion of public-inbox itself Archives are clonable: git clone --mirror https://public-inbox.org/meta git clone --mirror http://czquwvybam4bgbro.onion/meta git clone --mirror http://hjrcffqmbrq6wope.onion/meta git clone --mirror http://ou63pmih66umazou.onion/meta Example config snippet for mirrors Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.mail.public-inbox.meta nntp://ou63pmih66umazou.onion/inbox.comp.mail.public-inbox.meta nntp://czquwvybam4bgbro.onion/inbox.comp.mail.public-inbox.meta nntp://hjrcffqmbrq6wope.onion/inbox.comp.mail.public-inbox.meta nntp://news.gmane.io/gmane.mail.public-inbox.general note: .onion URLs require Tor: https://www.torproject.org/ AGPL code for this site: git clone https://public-inbox.org/public-inbox.git