we didn't check that the UIDs are adjacent, so we might have caught
not fetched deleted messages between two fetched messages below the
bulk load range.
checking adjacency of UIDs would make expunges in the bulk range (which
is likely to be full of holes) rather inefficient. so we use sequence
numbers instead.
this is admittedly a rather academical fix ...
amends 18225344.
try to purge sync entries based on which messages are *actually*
expunged, rather than those that are *expected* to be expunged.
to save network bandwidth, the IMAP driver doesn't report all expunges,
so some entry purges would be delayed - potentially indefinitely, e.g.,
when only --pull-new --push is used, or Trash isn't used (nor
ExpungeSolo, prospectively). so keep a fallback path to avoid this.
this is essentially the same as 'New', but for previously seen messages,
such as those that would have been instantly expunged (because they were
marked as deleted), those that we failed to store for some reason, and
already expired ones that are now flagged.
REFMAIL: CAOgBZNonT0s0b_yPs2vx81Ru3cQp5M93xpZ3syWBW-2CNoX_ow@mail.gmail.com
change it from --{push,pull} to --{new,gone,flags,upgrade}.
that alone doesn't change anything; combining it with any other option
would cause a complaint anyway.
this enables us to introduce --{push,pull}-full, which will matter when
--full doesn't actually include all type flags any more, as we'll then
be able to combine it with the extra type flag(s). in the same vein, we
now advertize "Sync Full" instead of "Sync All" (both continue to be
recognized).
this is more symmetrical with New, and results in some less dodgy
grammar. it also avoids confusion with the \Deleted flag.
fwiw, the pedantically correct name would be Expunges, but that's
confusingly close to the target-side expunge options. also, it's longer.
when propagation of too big messages was entirely suppressed, the only
way to force it was flagging the source message. however, now that we
have placeholders that can be flagged to trigger full propagation, it's
rather pointless to keep the old method working, and still doing it
does in fact confuse users, see for example
REFMAIL: CAOgBZNq_a9yKcq8Jw5y9VS6p2Se8mD7gkf6vPr_KU0taAWuGZQ@mail.gmail.com
to avoid this, we now almost completely shadow the regular meaning of
flagging - it basically becomes a non-synchronizable flag until the
placeholder is upgraded.
this was broken by commit de6dc699 - we now iterated only over far-side
messages, which we don't necessarily load, unlike the near-side ones
(which we need to do to know their current importance).
fix by iterating over sync entries instead of messages, which basically
restores the pre-19128f15 state in that regard. the minor catch here is
that we now need an auxiliary array to sort the sync entries by UIDs. on
the upside, we can also use it to avoid repeated calculation of the
flags.
make sure that an expiration calculation run is performed if ongoing
expiration transactions have been loaded from the journal. this has the
nice side effect of centralizing the regular trigger condition as well.
flags_set_p2() is also adjusted to query S_NEXPIRE only if an expiration
is actually ongoing, like the flag propagation submission loop already
did.
this went unnoticed, because expiration upon arrival of new messages
wasn't autotested at all - despite being the common case.
when the expiration is interrupted, but an external expunge removes the
near-side message before we resume, we would just drop the transaction,
resulting in an "unmarked" orphan.
a corner case is an expiration that finishes, but initially isn't
expunged (probably due to an interruption), followed by an unexpiration
that gets interrupted, and the message being expunged externally
(because it's still marked as deleted). we obviously can't complete
that transaction without re-propagating the message, so effectively
cancel it instead.
we cannot orphan all messages whose opposite we expunge, as that would
prevent subsequent propagation of the deletion. we can do that only if
the message is already known to be marked as deleted.
while we already refrained from propagating messages that would be
expunged from the target, we still propagated ones that would be
expunged from the source. this would lead to the weird situation of
creating orphans, and would pose journal replay idempotence problems.
such messages will now never have a sync record, so it becomes
pointless to test for S_PENDING in the trashing loop. note that the
behavior was previously bogus: these messages would have been paired by
the end of the run, so we shouldn't have treated them as solo for the
purposes of TrashOnlyNew/TrashRemoteNew.
we need to copy the S_DEL status bits when upgrading sync entries, so
that when a placeholder is flagged at the same time for both deletion
and upgrade, the entry pruning loop sees that the dummy message got
expunged.
note that we don't mask any bits, so the post-split entries may end up
with S_DEL for a zero UID - which doesn't matter.
don't implicitly propagate flags with upgrades. the user asked for
replacing the body, so do just that. if they also asked for flag
propagation, handle it like the case without upgrade as far as possible.
this makes async parallel flag propagation in the opposite direction
robust, while still being reasonably simple.
re-introduce newmaxuid, but now it's not used at all until the state
is committed. this simplifies the new-message loop, esp. in view of a
soon significantly increased number of branches in it.
the operation requires loading the target side, which makes it somewhat
expensive in otherwise unidirectional syncs.
one could also knock out Flags and Delete if there are no living pairs,
but that wouldn't actually save much.
instead of doing it instantly for every message, make a tally and
process it along with the OP_* flags. this allows us to print the
counts, and makes the handling more uniform.
started message propagations will now complete upon resumption even if
the run was requested to be a no-op - whether that's a good thing can
be argued, but it's the least effort way to avoid that committing
discards transaction state.
while this (currently) doesn't really matter (as all flag changes are
calculated before any are actually submitted), msg's flags should not
be updated before set_msg_flags() has actually succeeded.
as a side effect, this does away with the redundancy elimination and
pulling uid from msg (which were both unused since 19128f158).
instead of doing two runs for each journal entry, do one run for each
"write" operation, be it a journal entry or a writing driver call. this
saves runs between which no visible change occurred, which yields a 33%
improvement in runtime.
we now also exclude the final entry purge from the test, as it's really
kinda pointless, and we'd have to jump through additional hoops
(simulate an atomic commit of the state) to make it reliable in all
cases.
note that this also adds a few steps, which actually uncovered a bug in
the expunge sequencing.
amends efd72b85.
there is actually no use case overlap between between the two (though
limiting the step count does imply keeping the journal, as we exit
before we could commit anyway).
so far, we ensured that propagation _into_ a store completes before
expunging it, but not that propagation _from_ it completes. this way we
could end up expunging the source messages before the changes reached
the target, which could mess up resuming after an interruption.
this doesn't really fix anything under current conditions, as so far
only Maildir driver functions that modify known messages can cause
concurrent expunges to be detected, and we don't call any of these
before the so far unchecked loops.
so far, we checked for M_DEAD only in loops over messages. but we should
have checked srec->msg uses as well. this would make the code a mess, so
instead call back from the drivers when messages are expunged, so we can
reset the pointers.
the only case where this really matters so far is the flag setting loop,
which may cause the concurrent expunge of not yet handled messages to be
detected by the maildir driver.
all printing of auxiliary callback arguments must be conditional on the
command having actually succeeded. this affected fetch_msg() most, which
outright crashed due to a null pointer deref.
to fix this for good, we automate the generation of the status printing
and checking. as a side effect, this fixes the fetch_msg() callback not
printing the status at all.
amends 4cc5ad5a.
this tests only the common case of the far side being async - adding
100% instead of 50% to the runtime of the test to cover a corner case
didn't seem worth it.
instead of delaying the callback, delay the actual driver call. this is
in line with how the IMAP driver would behave, as since commit 6c08f568
it queues the socket writes (the network upstream latency goes on top,
but that doesn't alter the result).
amends 4423a932.
- print on which side we're upgrading a placeholder. as a side effect,
this moves some magic out of upgrade_srec().
- don't use past tense for something that is only about to happen
- don't print status of every sync entry affected by journal replay -
the entry load already prints it, and relevant operations print the
new status. also, it was inconsistent with not printing the entry's
old flags.
Oswald Buddenhagen