Make index rebuilds slightly more efficient by writing prepares from the same stream in a batch (DB-369)

[shaan1337]

Fixed: Make index rebuilds slightly more efficient by writing prepares from the same stream in a batch

Flags.HasAnyOf() bug

During index rebuilds at startup, this snippet of code had a bug but it turns out that it did not have any noticeable negative impact:

case LogRecordType.Prepare: {
        var prepare = (IPrepareLogRecord<TStreamId>)result.LogRecord;
        if (prepare.Flags.HasAnyOf(PrepareFlags.IsCommitted)) {
            if (prepare.Flags.HasAnyOf(PrepareFlags.SingleWrite)) {
                Commit(commitedPrepares, false, false);
                commitedPrepares.Clear();
                Commit(new[] { prepare }, result.Eof, false);
            } else {
                if (prepare.Flags.HasAnyOf(PrepareFlags.Data | PrepareFlags.StreamDelete))
                    commitedPrepares.Add(prepare);
                if (prepare.Flags.HasAnyOf(PrepareFlags.TransactionEnd)) {
                    Commit(commitedPrepares, result.Eof, false);
                    commitedPrepares.Clear();
                }
            }
        }

        break;
    }

The bug is that prepare.Flags.HasAnyOf(PrepareFlags.SingleWrite) should have been prepare.Flags.HasAllOf(PrepareFlags.SingleWrite) as SingleWrite is a composite flag: SingleWrite = Data | TransactionBegin | TransactionEnd

HasAnyOf returns true if any of the Data, TransactionBegin or TransactionEnd flags are present on the prepare log record.

Due to the bug, the else part in the if-else condition was never executed as described below:

• there are 3 cases to consider when IsCommitted flag is set: a normal event write, an empty write and a stream delete (tombstone)
• a normal event write has the Data flag, so the if block is entered
• an empty event write always has the TransactionBegin/TransactionEnd flags set, so the if block is entered
• a stream delete (tombstone) also always has the TransactionBegin/TransactionEnd flags set (together with the StreamDelete flag), so the if block is entered

Impact

The net effect is that:

• all prepares with the IsCommitted flag were committed to the index one at a time (this is not necessarily a bad thing in terms of performance as it would have happened for single writes anyway)
• for implicit transactions, it appears that the original code intended to commit events to the index transaction-wise but they were instead committed one at a time. Following Torn transactions (1/3): Prevent at chunk boundaries on the leader node (DB-64) #3808, Prevent implicit transactions from spanning multiple chunks (DB-66) #3918 and Prevent torn transactions during replication (DB-66) #3896, we now ensure that there is no torn transaction at startup in the log, so it is indeed fine to commit all the prepares of a transaction one at a time (without waiting for TransactionEnd)

This PR

The current code works fine but it is misleading due to the above bug. This PR rewrites this part to remove any ambiguity. It also takes the opportunity to add the events from the same stream in a batch to the index (it's not a very significant optimization but it's still better than committing to the index one at a time)

There should be no change in behaviour, except that:

• we now commit prepares of the same stream in batches of size up to 256
• Commit() is not called for empty writes (they are ignored in the Commit() method anyway)

Storage chaser initialization

Note that the index committer rebuilds the index from the index checkpoint up to the chaser checkpoint at startup - not the writer checkpoint. The remaining index rebuild (from the chaser to the writer checkpoint) is done by the storage chaser at startup. So if the chaser = writer at startup, then we can be sure that all events will be added to the index. but if chaser < writer and there is a torn transaction at the end of the log at startup, it will not be added by the storage chaser to the index (this can have unintended consequences such as duplicate event numbers). This is no longer a problem due to the 3 PRs (#3808, #3918 and #3896) that prevent torn transactions in the log. Note that there can still be a torn transaction at startup in the log when upgrading to the version with these fixes but the problem will no longer occur as from the next startup.

[linear]

DB-369 Fix HasAnyOfBug in IndexCommitter