Andrey Yarovoy created HDDS-17718:
-------------------------------------
Summary: Avoid protobuf TextFormat toString of DatanodeBlockID for
every readChunk when tracing is off
Key: HDDS-17718
URL: https://issues.apache.org/jira/browse/HDDS-17718
Project: Apache Ozone
Issue Type: Improvement
Components: Ozone Client
Reporter: Andrey Yarovoy
Assignee: Andrey Yarovoy
*Description*
`ContainerProtocolCalls.readChunk` adds span attributes on every call:
{code:java}
try (TracingUtil.TraceCloseable ignored =
TracingUtil.createActivatedSpan("readChunk")) {
Span span = TracingUtil.getActiveSpan();
span.setAttribute("offset", chunk.getOffset())
.setAttribute("length", chunk.getLen())
.setAttribute("block", blockID.toString());
{code}
`blockID` is a protobuf `DatanodeBlockID`, so `toString()` runs
`TextFormat.printer().printToString`: reflection over fields plus string
building. Java evaluates the argument even when tracing is disabled and the
span is the no-op `Span.getInvalid()`, whose `setAttribute` discards
it. `tryEachDatanode`'s `span.addEvent("... " + d)` has the same eager-string
pattern, but only on the error path.
*Evidence (HBase YCSB on EC, stream read OFF, RegionServer async-profiler,
6×60s)*
`AbstractMessage.toString` under `ContainerProtocolCalls.readChunk` costs
**0.7%** of RS CPU, about 4% of the classic read path, while tracing is
disabled. With stream read ON (`readChunk` unused) it is 0%.
*Proposed fix*
{code:java}
Span span = TracingUtil.getActiveSpan();
if (span.isRecording()) {
span.setAttribute("offset", chunk.getOffset())
.setAttribute("length", chunk.getLen())
.setAttribute("block", blockID.getContainerID() + ":" + blockID.getLocalID());
}
{code}
- Guard with `Span.isRecording()`, the OpenTelemetry API for "attributes will
be kept".
- Keep the attribute **name**. The value becomes the compact
`containerID:localID` form (BlockID convention) instead of multi-line
TextFormat, which
is cheaper and easier to read in traces even when tracing is on. If keeping the
exact old value matters, keep `toString()` inside the guard.
- Apply the same guard to `tryEachDatanode`'s `addEvent` calls, and check other
`span.setAttribute(..., x.toString())` sites in
`hadoop-hdds/client` and `hadoop-hdds/common`.
*Compatibility / side effects*
No wire change. With tracing on, the `block` attribute value format changes (if
the compact form is chosen).
*Tests*
- Unit: with tracing disabled, `readChunk` does not call
`DatanodeBlockID.toString()` (Mockito spy on the message or a counting `Span`).
With an
in-memory OpenTelemetry exporter and tracing enabled, the `readChunk` span
carries `offset`, `length` and `block`.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
