hudi-agent commented on code in PR #19575:
URL: https://github.com/apache/hudi/pull/19575#discussion_r3817095063
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/HoodieSparkTable.java:
##########
@@ -142,9 +146,25 @@ protected Option<HoodieTableMetadataWriter>
getMetadataWriter(
return Option.empty();
}
+ /**
+ * Carries the task thread's state onto the pool threads Hudi hands record
writing to.
+ *
+ * <p>The metrics binding travels alongside the {@link TaskContext} for the
same reason the
+ * TaskContext does: both are thread-local, and {@code
BoundedInMemoryExecutor} and
+ * {@code DisruptorExecutor} run on threads the task did not create. Without
this, a metric raised
+ * while writing records resolves against an unbound thread and is discarded.
+ *
+ * <p>Nothing unbinds afterwards. These pools are created and shut down per
operation, so the thread
+ * does not outlive the binding; a pool that were shared across tables would
need an unbind hook on
+ * the other side, which no current executor provides.
+ */
@Override
public Runnable getPreExecuteRunnable() {
final TaskContext taskContext = TaskContext.get();
- return () -> TaskContext$.MODULE$.setTaskContext(taskContext);
+ final Map<String, Registry> metricsBinding =
ExecutorMetricsContext.capture();
+ return () -> {
+ TaskContext$.MODULE$.setTaskContext(taskContext);
+ ExecutorMetricsContext.bind(metricsBinding);
Review Comment:
🤖 Since this override lives on the base `HoodieSparkTable`, every Spark
write's executor pool threads (merge/insert/clustering/bootstrap all go through
`getPreExecuteRunnable`) now get `bind(...)` and so report `isBound() == true`.
That flips `Registry.getRegistry(name)` on those threads: a name miss now
returns `NoOpRegistry` (discarded) instead of a process-wide `LocalRegistry`
(collected). Have you confirmed nothing on those pool threads emits via
`Registry.getRegistry`, so no unrelated metric gets silently dropped?
Relatedly, during the write phase the task thread isn't inside a lookup
`call()`'s bound scope, so `capture()` here looks like it returns an empty map
— is the propagation actually carrying the table's bundle as the javadoc
describes, or is it currently a no-op?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
