[v22.1.x] Fix duplicates consistency error by caching already translated offsets #5453
Conversation
rystsov
requested review from
dotnwat,
NyaliaLui,
mmaslankaprv,
graphcareful,
VadimPlh and
ztlpn
as code owners
|
There were multiple conflicts on cherry-picking |
|
Had to omit cherry-picking upgrade testing because the upgrade testing dependencies are not backported |
mmedenjak
added
kind/bug
(cherry picked from commit 7862d4b) Conflicts: src/v/cluster/controller.cc src/v/cluster/controller.h src/v/redpanda/application.cc
Kafka client doesn't process unknown_server_error correctly and it may lead to duplicates violating the idempotency. See the following issue for more info: https://issues.apache.org/jira/browse/KAFKA-14034 request_timed_out just like unknown_server_error means that the true outcome of the operation is unknown and unlike unknown_server_error it doesn't cause the problem so switching to using it to avoid the problem (cherry picked from commit 1a72446)
(cherry picked from commit 1dfd8d9)
Update all partition::replicate dependees which don't perform offset translation to bypass it via a direct raft reference (cherry picked from commit 67a3112) Conflicts: src/v/cluster/partition.h src/v/cluster/partition_probe.cc src/v/kafka/server/group.cc
|
The sketchiest change is in feature_manages: it uses logical version which should be consistent with releases so if a feature (in this case |
Updating consumer groups to use conditional replication to prevent a situation when after a check a leadership jumps away, invalidates the check, jumps back just in time for the post check replication. check condition leadership goes to a new node the node replicates something which invalidates the conditions the leadership jumps back the node successfully replicates assuming that the condition is true Switched to a conditional replicate to fix the problem. When a group manager detects a leadership change it replays the group's records to reconstruct the groups state. We cache the current term in the state and use it as a condition on replicate. In this case we know that if the leadership bounce the replication won't pass. (cherry picked from commit e693bea) Conflicts: src/v/kafka/server/group.cc
We're going to mix raft and kafka offset in the same class, since both the offsets uses the same type it's easy to make an error and treat one as it was another. Introducing kafka offset to rely on the type system to prevent such errors. (cherry picked from commit 9335075) Conflicts: src/v/cluster/types.h
Shifting offset translation down the abstraction well to eventually reach rm_stm (cherry picked from commit e3d24d9)
Preparing rm_stm to use kafka::offset based seq-offset cache. Right now it uses raft offsets but there is a problem with it: once the cache items become older that the head of the log (eviction) panda becomes unable to use offset translation so we need to store already translated offsets. Since the cache is persisted as a part the snapshot so we need to change the disk format and provide backward compatibility. The change is splitted into two commits. Current commit introduces types to represent old format seq_cache_entry_v1 and tx_snapshot_v1 and adds compatibility machinary to convert old snapshot (tx_snapshot_v1) to new snapshot (tx_snapshot). The follow up commit updates the default types to use new format and updates the mapping between old and default types. (cherry picked from commit 63c5883) Conflicts: src/v/cluster/rm_stm.cc
switching to caching seq-kafka offsets cache to avoid out of range errors on translating offsets beyond the eviction point (cherry picked from commit 4b42c7e)
(cherry picked from commit 065fb54)
(cherry picked from commit 9000762)
(cherry picked from commit 8e7346d) Conflicts: src/v/cluster/feature_table.cc src/v/cluster/feature_table.h tests/rptest/tests/cluster_features_test.py
Looks risky but I guess we don't have an option here. What you are suggesting makes sense to me. I skimmed through the feature tables on the branches (21.1.x, 22.1.x, dev) and the only conflict seems to be on dev where we need to bump |
There was a problem hiding this comment.
lgtm.
I'm not sure what the policy is around backporting new features but in this case this is a 'bugfix' which needs a serialization change and this is blocking 2.1.5 release.
@jcsp You might be interested in this given it's implications on the feature table in the dev branch. Just FYI.
Backport from pull request #5356, #5220
fixes #5452