[server] Add pre-write buffer memory metrics for primary key tables - #4308
Kaixuan-Duan wants to merge 5 commits into
Conversation
|
looks duplicate with #3533 |
I've looked at both #4308 and #3533. IMO, the way #4308 calculates actual memory usage reflects the true memory footprint — it factors in the real overhead of Java objects, which I believe is the correct approach. @zuston, do you still plan to move forward with #3533? If so, I'd suggest adjusting it to follow the approach taken in this PR. Additionally, I think the unified memory accounting introduced in #3751 should be incorporated into this PR. To be more specific: we need a truly accurate global memory usage ledger for KvPrewriteBuffer, maintained by atomic variables. Our future backpressure decisions will depend on it, and the current metrics should also be built on the same source of truth — rather than being the sum of non-atomic samples collected from multiple maps by the metrics framework. |
make sense, please go ahead @Kaixuan-Duan . And I will close the #3533 |
77b1e04 to
c1806c7
Compare
|
@platinumhamburg @zuston Thanks for the direction, and for closing #3533 |
bdd9126 to
323b774
Compare
323b774 to
5c94ad5
Compare
The put path updated the server-wide shared AtomicLong once per record, leaving it the only per-record RMW on a counter contended by concurrent writers across tablets. Accumulate the accounting delta locally in the kv write lock and publish it once per write batch, symmetric with the already-batched flush/truncate release. The batch boundary is the end of putAsLeader (try/finally, so partial failures still publish) and the end of log replay in KvRecoverHelper. A brief conservative under-count while a batch is pending is acceptable for an observability metric. close() settles the remaining accounting, including any unpublished delta, in a single update and stays idempotent.
3 participants
Purpose
Linked issue: close #4307
Brief change log
Tests
API and Format
Documentation