Michael Westerby created KAFKA-21062:
----------------------------------------

             Summary: Potential data loss during KRaft migration due to a lost 
ZooKeeper acknowledgement on /migration write
                 Key: KAFKA-21062
                 URL: https://issues.apache.org/jira/browse/KAFKA-21062
             Project: Kafka
          Issue Type: Bug
          Components: kraft, migration, zkclient
    Affects Versions: 3.9.2
            Reporter: Michael Westerby


A lost ZooKeeper acknowledgment on a successful {{/migration}} znode write can 
cause the KRaft migration driver to mistake its own prior write for a 
conflicting one, permanently stalling it once it reaches the {{DUAL_WRITE}} 
stage. This prevents further writes to ZooKeeper, allowing the state in between 
KRaft and ZooKeeper to diverge during the migration.

This happens because the client the migration uses to talk to ZooKeeper will 
retry an identical request to {{/migration}} if it sees a connection loss. If 
the original request had actually already succeeded on the server, but only its 
acknowledgment was lost, the retry sends the same expected version of the 
{{/migration}} znode again. Since that version has already moved on, ZooKeeper 
rejects the retry as a version conflict, even though nothing is actually wrong.

The migration driver currently has no way to tell this apart from a genuine 
conflict caused by a second controller writing at the same time, so it treats 
every such rejection as a hard failure. Because the write is treated as a 
failure, the driver's own record of what version the {{/migration}} znode is 
currently at never gets corrected, and stays at the old, stale value. Every 
subsequent write then reuses that same stale value and hits the same rejection, 
indefinitely. The driver becomes permanently stuck, unable to make further 
migration progress, until an unrelated event, such as a new controller 
election, forces an unconditional resynchronization of {{{}/migration{}}}.

While stuck, ZooKeeper stops receiving updates from the controller, and any 
ZK-mode brokers that rely on ZooKeeper for their view of cluster state are 
never notified of the corresponding changes via RPCs either. This causes the 
states in ZooKeeper and KRaft to diverge, which can destabilize the cluster 
further. 

This is of particular concern when some, but not all, brokers have already been 
restarted into their KRaft mode. The remaining ZK-mode brokers are left running 
on an increasingly outdated view of the cluster state. In the worst case, this 
can cause real data loss if the KRaft side elects a new partition leader while 
the driver is stuck. As that change is never propagated to ZooKeeper, and the 
ZK-mode brokers are never notified of it via RPC either, the old leader can 
continue to operate under the stale, lower epoch. The old leader can keep 
accepting and acknowledging produce requests it is no longer authorized to 
serve. When it eventually discovers the true, higher-epoch leader, it must 
truncate its own log to reconcile, silently discarding any writes it had 
already acknowledged to producers in the meantime.

h2. Walkthrough

[{{KafkaZkClient}} 
|https://github.com/apache/kafka/blob/3.9.2/core/src/main/scala/kafka/zk/KafkaZkClient.scala]exposes
 two methods used to write to the {{/migration}} znode in ZooKeeper during a 
ZK-to-KRaft migration: 
 # the standalone 
[{{updateMigrationState}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/zk/KafkaZkClient.scala#L1767]
 # the bundled 
[{{{}retryMigrationRequestsUntilConnected{}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/zk/KafkaZkClient.scala#L2012].

Both take a 
[{{ZkMigrationLeadershipState}}|https://github.com/apache/kafka/blob/3.9.2/metadata/src/main/java/org/apache/kafka/metadata/migration/ZkMigrationLeadershipState.java]
 and send its {{migrationZkVersion}} field to ZooKeeper as the expected version 
for a conditional write to {{{}/migration{}}}.

Both methods ultimately use {{{}KafkaZkClient{}}}'s own retry loop 
[({{{}retryRequestsUntilConnected{}}})|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/zk/KafkaZkClient.scala#L2123],
 which repeatedly calls into the lower-level {{ZooKeeperClient}} and resends 
whenever it sees {{{}CONNECTIONLOSS{}}}. If the connection is lost 
({{{}ConnectionLossException{}}}) after a write has already committed 
server-side, but before its acknowledgment reaches the client, this loop 
transparently resends the identical request, still carrying the same 
{{{}migrationZkVersion{}}}, with no way of knowing the original attempt already 
succeeded. Since the version has genuinely moved on, this resend is rejected 
with {{{}BadVersionException{}}}, even though nothing is actually wrong.

The two methods surface this differently:
 # {{updateMigrationState}} issues a single {{{}SetDataRequest{}}}. On the 
resend's {{{}BadVersionException{}}}, this propagates directly out via 
{{{}resp.maybeThrow(){}}}.
 # {{retryMigrationRequestsUntilConnected}} wraps each request in a multi-op 
transaction that includes a {{{}CheckOp{}}}/{{{}SetDataOp{}}} on 
{{{}/migration{}}}. On the resend's {{{}BadVersionException{}}}, 
{{handleUnwrappedMigrationResult}} throws an unconditional 
{{{}RuntimeException{}}}, assuming a second KRaft controller must be writing to 
ZooKeeper.


Both methods are called from the 
[{{{}KRaftMigrationDriver{}}}|https://github.com/apache/kafka/blob/3.9.2/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java],
 which caches the {{ZkMigrationLeadershipState}} (and its 
{{{}migrationZkVersion{}}}) in {{{}this.migrationLeadershipState{}}}. Once the 
driver reaches {{{}DUAL_WRITE{}}}, 
[{{MetadataChangeEvent.run()}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L510]
 calls into them continuously, from three places:
 # 
[{{{}handleDelta{}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L574],
 for each incremental metadata change, writes topic/config/ACL/delegation-token 
updates via {{{}retryMigrationRequestsUntilConnected{}}}.
 # 
[{{{}handleSnapshot{}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L570],
 the same mechanism, used when a full metadata snapshot is loaded instead of an 
incremental delta.
 # The dedicated checkpoint write at the end of the same event, 
[{{{}zkMigrationClient.setMigrationRecoveryState(zkStateAfterDualWrite){}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L593],
 via {{{}updateMigrationState{}}}.


Each of these calls is wrapped in {{{}applyMigrationOperation{}}}, which only 
[reassigns {{this.migrationLeadershipState}} when the call returns 
{*}successfully{*}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L257].
 As these methods will now always throw an exception, this leaves the driver's 
cached {{migrationZkVersion}} stale, and is never corrected. Every subsequent 
write, from any of the three call sites, reuses that same stale version and 
fails identically, so the driver becomes stuck indefinitely, unable to make 
further migration progress. 

Both exceptions will eventually surface at 
[{{MigrationEvent.handleException}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L416]
 : 


{code:java}
        public void handleException(Throwable e) {
            if (e instanceof MigrationClientAuthException) {
                KRaftMigrationDriver.this.faultHandler.handleFault("Encountered 
ZooKeeper authentication in " + this, e);
            } else if (e instanceof MigrationClientException) {
                log.info(String.format("Encountered ZooKeeper error during 
event %s. Will retry.", this), e.getCause());
            } else if (e instanceof RejectedExecutionException) {
                log.debug("Not processing {} because the event queue is 
closed.", this);
            } else {
                KRaftMigrationDriver.this.faultHandler.handleFault("Unhandled 
error in " + this, e);
            }
        }
{code}

 # The {{MigrationClientException}} which wraps {{updateMigrationState}} ‘s 
{{BadVersionException}} (via 
[{{{}ZkMigrationClient.wrapZkExeception{}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/zk/ZkMigrationClient.scala#L68])
 is simply logged at INFO level as “will retry”.
 # The unwrapped {{RuntimeException}} from 
{{retryMigrationRequestsUntilConnected}} instead falls through to 
{{KRaftMigrationDriver.this.faultHandler.handleFault(...)}} . The controller 
which builds that fault handler provides it with [{{fatal = 
false}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/server/ControllerServer.scala#L304]
 in 
[{{ControllerServer.scala}}|https://github.com/apache/kafka/blob/3.9.2/core/src/main/scala/kafka/server/ControllerServer.scala]
 which when provided to the {{StandardFaultHandlerFactory}} will create a 
{{LoggingFaultHandler}} . This will simply log exceptions as errors, without 
terminating the process.


{code:java}
        val migrationDriver = KRaftMigrationDriver.newBuilder()
          ...  
          .setFaultHandler(sharedServer.faultHandlerFactory.build(
            "zk migration",
            fatal = false,
            () => {}
          ))
{code}


Given both paths result in errors which are just logged, nothing actually 
causes the process to exit fatally, meaning the current driver will persist in 
this stalled state indefinitely. It only recovers once an unrelated event (e.g. 
a new controller election, which performs an unconditional overwrite of 
{{{}/migration{}}}) happens to resynchronize it.


This {{ConnectionLossException}} / {{BadVersionException}} pattern has been 
seen before, with an existing precedent on how to address it. 
[{{KafkaZkClient.conditionalUpdatePath}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/zk/KafkaZkClient.scala#L965]
 supports an optional checker function that, on a {{BadVersionException}} , 
re-reads the znode and compares its content against what the caller intended to 
write, so a genuine conflict can be told apart from the caller’s own retried 
write having already landed. 
[{{ReplicationUtils.updateLeaderAndIsr}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/utils/ReplicationUtils.scala#L33-L34]
 already rely on this via 
[{{checkLeaderAndIsrZkData}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/core/src/main/scala/kafka/utils/ReplicationUtils.scala#L38]
 for leader/ISR updates. However, neither the {{updateMigrationState}} nor 
{{retryMigrationRequestsUntilConnected}} currently use it for 
{{{}/migration{}}}.


h2. ZkWriteBehindLag Metric

When the controller is in this stuck state, the {{ZkWriteBehindLag}} metric can 
fail to report the actual lag present. The metric works by comparing the 
controller's latest committed offset against the offset it believes it has 
already mirrored to ZooKeeper, but that mirrored offset is recorded before the 
checkpoint write to {{/migration}} is attempted, not after it succeeds. On an 
event that doesn't otherwise involve any topic, config, ACL, quota, producer 
ID, or delegation token changes, such as one driven by a no-op record, this 
recording happens regardless, immediately before the checkpoint write that then 
fails. The metric can therefore report at or near zero lag even while the 
driver is completely stuck, giving operators no indication that dual-write has 
failed.

The {{ZkWriteBehindLag}} gauge in 
[{{QuorumControllerMetrics}}|https://github.com/apache/kafka/blob/3.9.2/metadata/src/main/java/org/apache/kafka/controller/metrics/QuorumControllerMetrics.java]
 is computed as
[{{{}lastCommittedRecordOffset() - 
dualWriteOffset(){}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/controller/metrics/QuorumControllerMetrics.java#L167].
 {{dualWriteOffset}} is updated via
[{{{}controllerMetrics.updateDualWriteOffset(image.highestOffsetAndEpoch().offset()){}}}|https://github.com/apache/kafka/blob/5e9866f43ab8e7e41ef39e5584ac50019381328d/metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java#L590],
 called in
{{MetadataChangeEvent.run()}} immediately before the checkpoint write described 
above.

Whether that update is actually reached, for any given event, depends on what 
the event contains:
 * If the event includes any topic, config, client quota, SCRAM credential, 
producer ID, ACL, or delegation token change, 
{{KRaftMigrationZkWriter.handleDelta}} calls into the bundled write path 
({{{}retryMigrationRequestsUntilConnected{}}}) for each one. Once the driver is 
already stuck, these calls fail the same way as described above, and the 
exception propagates out of {{handleDelta}} before {{updateDualWriteOffset}} is 
ever reached. The metric correctly stays where it was, and the reported lag 
keeps growing as expected.
 * If the event contains none of those changes (for example, one driven only by 
a broker registration change or a {{{}NoOpRecord{}}}), {{handleDelta}} has 
nothing to write, returns normally, and execution reaches 
{{updateDualWriteOffset}} regardless. {{dualWriteOffset}} advances to the 
latest offset, and only then does the checkpoint write immediately after it 
fail. On these events, the lag metric is misleadingly reset toward zero, even 
though the driver is still stuck.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to