HhH666-ux commented on PR #40097: URL: https://github.com/apache/shardingsphere/pull/40097#issuecomment-6465125576
Pushed `6c805be`, which rebuilds the abandonment mechanism and closes the remaining rule findings. **1. [P2] Reclamation ends spans belonging to active queries — the deadline is gone.** Agreed: no elapsed-time bound can separate a running invocation from one whose completion hooks were skipped, because JDBC allows a statement with no query timeout. The executor skips those hooks in exactly one situation — `isPluginEnabled` is false when `adviceAfter` runs — so the plugin enablement now carries the decision instead of the clock. `PendingSpan` records the generation it was registered under. When the check observes the plugin disabled it advances the generation once and releases only spans from an earlier generation; a span whose generation is still current is never touched, so a query that runs for hours keeps its span and its eventual exception. Spans still pending when the plugin stops are released from `OpenTelemetryTracingPluginLifecycleService.close()`, so the retention of an abandoned span stays finite without a deadline ever cutting a running invocation short. Regression tests: `assertRunningInvocationKeepsItsSpan` drives the check while an invocation is in flight, asserts that the span is neither exported nor removed, then asserts that the invocation still ends its own span as `OK`. `assertPendingSpanIsReleasedWhenPluginStops` covers the release path through the plugin lifecycle. **2. [P3] The test invokes a nonpublic production helper — fixed.** The test no longer calls the release routine. The check is driven through the public `endAbandonedSpans()`, and the release path through `OpenTelemetryTracingPluginLifecycleService.close()`. **3. [P3] PendingSpan annotations reverse the required order — fixed.** `@RequiredArgsConstructor` now precedes `@Getter`. **4. [P3] The empty-map assertion violates the assertion rule — fixed.** Both sites now use `assertTrue(getPendingSpans().isEmpty())`. **5. [P3] The range comparison places its constant on the wrong side — the comparison is gone.** The generation comparison replaced it. **6. [P3] The scheduler invocation wraps before necessary — no longer applicable.** The call is now the declaration line ending in the method reference, with the two interval arguments on the continuation line; the three arguments together reach 214 columns. Items 3 and 4 were present in the previous revision and I missed them earlier — both are in this commit. I re-read the cited rule files against the current head rather than only against the diff, and the module build is left to CI as the first full verification of this commit. -- 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]
