zhixinwen commented on issue #3070: URL: https://github.com/apache/kvrocks/issues/3070#issuecomment-3105010911
> ``` > replication_ack_duration = inf > replication_ack_seq_threshold = 0 > ``` > This would have an issue. If duration is inf and write stops before `replication_ack_seq_threshold` is met, the replica would never send ack for those writes. Also, replication_ack_seq_threshold is not 100% the same as the current impl. In the current `IncrementBatchLoop` call, there can be multiple write batches processed, it would only send ack once in `IncrementBatchLoop`. If I understand the algorithm correctly, `replication_ack_seq_threshold=0` would make one function call send multiple acks which can further hurt performance. I think a slight change should work: ``` // if replication_ack_seq_buffer = 0, this value is ignored and can be unset (-1) // otherwise this value must be set replication_ack_interval // -1 means disabled and rely on replication_ack_interval only // 0 means the processes all of the data in `IncrementBatchLoop` call and send ack. // N >= 1 means ack after N changes replication_ack_seq_buffer ``` The current unstable branch behavior is equivalent to: ``` replication_ack_interval: -1 replication_ack_seq_buffer: 0 ``` By default, we can set it as: ``` replication_ack_interval: 15s // something close to metrics scrape interval replication_ack_seq_buffer: -1 ``` so it would minimize the ack overhead. -- 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]
