Conversation
swuferhong
left a comment
There was a problem hiding this comment.
Hi, @fhan688 I suggest extracting an internal WriteThrottleController to unify the existing disk-write backoff and KV backpressure handling:
- Policies translate errors or pressure signals into a throttling reason, scope, and deadline.
- The controller maintains state separately for each reason, computes the effective deadline, handles cleanup, and wakes the Sender when the next eligible send time moves earlier.
- ready() queries the remaining delay, and drain() checks eligibility again before draining batches. Adding a new policy should not require changes to either path.
- If we later need QPS or bandwidth limits, we can introduce non-blocking permit acquisition before sending requests. If the goal is simply to suppress aggressive retries during write rejection, we could allow only a bounded number of probe requests after the backoff expires.
This would make disk-write backoff one policy within a reusable write-throttling mechanism and provide a natural extension point for future throttling scenarios.
I'd like to confirm that extracting an internal |
I'd prefer to do it in this PR: set up the architecture properly upfront and reduce the amount of refactoring needed later. WDYT @platinumhamburg |
Agree +1. |
c04efc1 to
60c2dc3
Compare
When disk protection rejects a write with DISK_WRITE_LOCKED, the Log/KV
writers apply a bounded exponential backoff before retrying that bucket,
instead of hot-looping and hammering the rejecting server.
Write-throttling is unified in a new WriteThrottleController that owns two
independent gates per bucket:
- KV backpressure (wall-clock, quadratic) for KV upsert/delete
- disk-write backoff (monotonic nanos, exponential, never-shorten,
ceil-to-ms) for DISK_WRITE_LOCKED rejections
Per bucket the effective delay is max(kvRemainingMs, diskRemainingMs); the
accumulator wakes on the min across buckets. bucketReady keeps leader-first
ordering: a leaderless bucket is still reported to unknownLeaderTables while
a gate is pending, so metadata refresh (which sends no data) stays orthogonal
to throttling.
Adds ConfigOptions for the backoff bounds, documentation, unit coverage in
RecordAccumulatorTest/SenderTest, and DiskWriteBackoffITCase.
60c2dc3 to
a0f8439
Compare
Purpose
Disk protection rejects writes with the
DISK_WRITE_LOCKEDerror code. Today the Java client immediately retries and resends the full payload, generating excessive network traffic and consuming client CPU without making progress during sustained disk pressure.This PR introduces bounded exponential backoff for the Java Log and KV writers. It targets the Java client only; Rust-client support is planned separately.
Linked issue: #4278
Brief change log
client.writer.disk-write-locked.backoff— initial backoff, default1sclient.writer.disk-write-locked.backoff-max— maximum backoff incl. jitter, default10s1ms <= initial <= maximum <= 2147483647ms.DISK_WRITE_LOCKEDin the sharedSenderretry path; track backoff deadlines by destinationTableBucket; enforce backoff in readiness checks and batch draining; preserve retry limits; wake theSenderafter re-enqueueing.@Internal WriteThrottleControllerthat unifies the two per-bucket write-throttling gates — KV backpressure (wall-clock millis) and disk-write backoff (monotonic nanos) — behind a single read side:remainingDelayMs(tb) = max(kvRemainingMs, diskRemainingMs)andisGated(tb). Both sendability paths,ready()anddrain()'sshouldSkipBucket(), now consult the controller, so no single gate can be dropped. Within a bucket the gates fold viamax(); across bucketsready()keeps the earliest wake-up viamin(). This implements the refactor@swuferhong requested (agreed in-thread to land in this PR). The public surface and existing throttle/backoff behavior are unchanged.
bucketReady(), a bucket whose leader is unknown is still reported tounknownLeaderTableseven while a throttle gate is pending. A metadata refresh is orthogonal to the gate (itsends no data), and refreshing now means the leader is ready to receive the moment the gate clears.
Tests
Validated with JDK 11.0.27 and Maven 3.8.6.
RecordAccumulatorTest: 27 tests pass (was 25; added coverage for the unified gate and disk/KV independence, and removed one interim test that assumed gate-first ordering).SenderTest: 51 tests pass.DiskWriteBackoffITCase: 3 tests pass.BUILD SUCCESS(checkstyle / spotless / apache-rat / enforcer all green).Local traffic comparison (single writer / bucket / TabletServer, ~5s simulated protection):
These are single local samples and do not establish production capacity.
API and format changes
Adds two Java-client config options only. Reuses the existing
DISK_WRITE_LOCKEDerror code; no server protocol upgrade required, but the Java client must be upgraded. Backoff applies per physical table bucket.Documentation
Updates
website/docs/maintenance/configuration.mdwith the two new options.