voonhous opened a new issue, #19759:
URL: https://github.com/apache/hudi/issues/19759

   ### Describe the problem
   
   Follow-up to #19063, which scopes the process-wide `Registry.REGISTRY_MAP` / 
`HoodieSparkEngineContext.DISTRIBUTED_REGISTRY_MAP` refactor out of Phase 1 and 
lists "tying registry lifetime to `HoodieEngineContext` / write client rather 
than the JVM process" as a separate issue. #19575 (record index lookup 
counters) is the first consumer of the commit-boundary drain and deferred the 
items below here. Pointers are against #19575 head `0a9d4b412cae`.
   
   ### Checklist
   
   **A. Registry lifetime (the refactor #19063 defers)**
   
   - [ ] Make the registry an instance field on `HoodieSparkEngineContext` 
keyed by base path, with `ExecutorMetrics.publish` reading it off `context` 
instead of `Registry.REGISTRY_MAP`. Removes the SHA-256 base-path digest in 
`ExecutorMetricRegistry.scopedName`, the stale-context and replacement branches 
in `getMetricRegistry`, and `removeMetricRegistry`. (#19575 
https://github.com/apache/hudi/pull/19575#discussion_r3853269077)
   - [ ] Keep executor-metric registries out of the `DISTRIBUTED_REGISTRY_MAP` 
capture in `HoodieSparkEngineContext.map/mapToPair/flatMap`: every job for 
every table in the JVM ships one accumulator per RLI table per task, and 
nothing evicts (40+ entries after one functional run). 
(https://github.com/apache/hudi/pull/19575#discussion_r3853269022)
   - [ ] `Metrics.shutdown()` -> `registerHoodieCommonMetrics()` -> 
`Registry.getAllMetrics(true, true)` scrapes every registry in the process-wide 
map; the RLI registry's dotted name skips the common prefix, so leftover 
counters (abandoned commit, straggler task) are published as 
`<prefix>.<table>.HoodieRecordIndexLookup.<12hex>.<metric>` and then cleared. 
Goes away once the registry is outside `REGISTRY_MAP`. 
(https://github.com/apache/hudi/pull/19575#discussion_r3853269153)
   - [ ] The `!(registry instanceof DistributedRegistry)` fallback in 
`getMetricRegistry` is uncovered (`TestDistributedRegistry` names it but never 
enters it); drop it with the map. 
(https://github.com/apache/hudi/pull/19575#discussion_r3853269168)
   
   **B. Executor-side lookup API (hudi-io surface added by #19575)**
   
   - [ ] Decide whether name resolution stays: `Registry.getRegistry(name)` via 
the `ExecutorMetricsContext` thread-local binding, `NoOpRegistry`, the 
`Registry.getRegistry` contract change, and the 
`HoodieSparkTable.getPreExecuteRunnable` binding. Both shipped emitters hold 
the bundle by closure, so passing the `Registry` in directly removes about 330 
lines with identical behaviour. 
(https://github.com/apache/hudi/pull/19575#discussion_r3853269071, 
https://github.com/apache/hudi/pull/19575#discussion_r3817095063)
   - [ ] `Registry.release` interface default and the `LocalRegistry` override 
are unreachable in production (only `ExecutorMetrics` calls `release`, always 
on a `DistributedRegistry`). Drop or test. 
(https://github.com/apache/hudi/pull/19575#discussion_r3853269157)
   
   **C. Cleanups**
   
   - [ ] Delete the dead `HoodieMetadataMetrics.LOOKUP_RECORD_INDEX_*` 
constants (unused since `2a0d2aeef6a0` / HUDI-7391) and the `TODO [HUDI-9544]` 
in `HoodieBackedTableMetadata`, which #19575 supersedes. 
(https://github.com/apache/hudi/pull/19575#discussion_r3853269047)
   - [ ] Caller attribution (`tag` vs `dedupe`) was removed from #19575 for 
lack of coverage; restore only with a DataSource `insert.drop.duplicates=true` 
test, if the dedupe path is worth it. 
(https://github.com/apache/hudi/pull/19575#discussion_r3853268998)
   
   Not in scope here: #19063 gap 3 (counters leak across write-client 
lifetimes) is what #19575 thread 
https://github.com/apache/hudi/pull/19575#discussion_r3853268971 hits on the 
SQL and StreamSync paths; that belongs in #19575.
   


-- 
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]

Reply via email to