cauchy1988 opened a new pull request, #2408:
URL: https://github.com/apache/incubator-pegasus/pull/2408
## Problem
`pegasus_mutation_duplicator::duplicate()` iterates the mutation set (sorted
by timestamp), accumulating entries into a batch and flushing it when the
batch-byte threshold is reached **or the last mutation is seen**. Mutations
whose rpc code is `RPC_RRDB_RRDB_DUPLICATE` or `RPC_RRDB_RRDB_BULK_LOAD` are
intentionally not duplicated.
The ignored-code path used `continue`, which also skipped the batch-flush
check. So when the **last** mutation in the set was an ignored code, the
in-progress batch of valid mutations was never moved to `_inflights` and was
silently dropped. Because the caller has already advanced `last_decree` past
the whole set, those mutations were never re-read and never duplicated —
**silent data loss** (reported in #2284, with `duplicate_log_batch_bytes > 0`).
```cpp
for (auto mut : muts) {
batch_count++;
...
if (gutil::ContainsKey(ingnored_rpc_code, rpc_code)) {
...
continue; // <-- skips the flush check below
}
... add entry to batch ...
if (batch_count == muts.size() || batch_bytes >=
FLAGS_duplicate_log_batch_bytes || ...) {
... flush batch to _inflights ... // <-- never reached if the last
mutation was ignored
}
}
```
## Fix
1. Replace `continue` with `if/else` so the flush check **always** runs —
the trailing batch is now flushed even when the last mutation is ignored.
2. Flush only when the batch is non-empty (an ignored mutation with an empty
trailing batch is a no-op, preserving the all-ignored → `cb(0)` path covered by
the existing `duplicate_duplicate` test).
3. Compute the batch's routing hash from the last **entry** in the batch
(always a duplicatable code: `PUT`/`REMOVE`/`MULTI_*`), not the current
(possibly ignored) mutation — `get_hash_from_request` would `LOG_FATAL`-abort
on `DUPLICATE`/`BULK_LOAD`.
## Testing
Added regression test `duplicate_ignored_last_mutation`: 3 valid `PUT`s
followed by an ignored `BULK_LOAD` with the highest timestamp (iterated last).
It fails before the fix (`mail_box` empty — the trailing PUT batch is dropped)
and passes after.
Full duplicator suite passes (16/16):
```
[ RUN ]
pegasus_mutation_duplicator_test.duplicate_ignored_last_mutation/0
[ OK ]
pegasus_mutation_duplicator_test.duplicate_ignored_last_mutation/0 (1 ms)
[ RUN ]
pegasus_mutation_duplicator_test.duplicate_ignored_last_mutation/1
[ OK ]
pegasus_mutation_duplicator_test.duplicate_ignored_last_mutation/1 (2 ms)
...
[==========] 16 tests from 1 test suite ran. (6130 ms total)
[ PASSED ] 16 tests.
```
Existing tests (`duplicate`, `duplicate_failed`,
`duplicate_isolated_hashkeys`, `duplicate_duplicate`, `get_hash_from_request`,
`read_after_get_hash_key`, `create_duplicator`) all still pass — no regression.
Closes #2284
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
