Repository navigation
[Feature] decouple json-rpc filter processing from Manager #6963
Description
Summary
The json-rpc filter event-processing thread currently is located within Manager (core layer), forcing it to hold TronJsonRpcImpl (API layer) via a @Lazy injection — a reverse circular dependency. This proposal moves that thread into TronJsonRpcImpl; Manager only posts filter events into a standalone queue bean, and the reverse edge disappears.
Problem
Motivation
#6732 turned handleLogsFilter/handleBLockFilter from static into instance methods, so Manager's event-processing thread needs a TronJsonRpcImpl instance; the review deferred the resulting @Lazy cycle to a follow-up (discussion) — this issue serves as the dedicated follow-up to address that technical debt.
Current State
Manager ──@Lazy @Autowired field──▶ TronJsonRpcImpl ──@Autowired field──▶ Manager
Each arrow reads "depends on"; the labels are injection styles. TronJsonRpcImpl's constructor parameters Wallet/NodeInfoService each depend on Manager as well, closing the same cycle indirectly.
Manager owns the filter queue, the consumer thread, and its shutdown; the consumer calls back into the API layer.
Limitations or Risks
FullNodesetsallowCircularReferences(false), so a directly injected cycle is rejected at startup;@Lazy(likeObjectProvideror a runtimegetBean()) only makes Spring tolerate the cycle — the reverse edge stays, obscuring the component graph and complicating lifecycle changes.- The reverse core → API edge risks encouraging future changes to repeat the same
@Lazypattern, widening the breach. - One latent defect remains in the current implementation: the consumer stop flag is a non-volatile boolean (the shutdown-time write has no visibility guarantee for that thread), and the loop does not exit on
InterruptedException— under the Java memory model, neither the normal nor the forced stop path is reliable.
Proposed Solution
Proposed Design
Eliminate the reverse dependency entirely rather than suppressing it via @lazy: a standalone FilterCapsuleQueue bean is introduced as an intermediary between the two layers — Manager only produces into it; TronJsonRpcImpl owns the consumer thread (start on @PostConstruct, stop via idempotent close()). ApplicationImpl.shutdown() calls close() explicitly after producers stop and before dbManager.close() — Spring destroys beans only after the database closes, so bean destruction alone would let the consumer outlive it (ApplicationImpl is already the shutdown-orchestrating composition root). Filter API behavior stays unchanged (unbounded queue; pending entries are not drained on shutdown, while an in-flight capsule may complete).
Key Changes
- Module:
frameworkonly org.tron.common.logsfilter.queue.FilterCapsuleQueue(new bean, beside the existingorg.tron.common.logsfilter.capsulepackage)Manager: drop the@Lazyfield, the consumer loop, and its thread lifecycleTronJsonRpcImpl: constructor gains aManagerparameter, consumer lifecycle, rewrittenclose()ordering (stop and await the consumer first, then shut downlogsFilterPool)ApplicationImpl.shutdown(): explicit consumer stop- No configuration or API changes
Impact
- Stability: fixes the latent stop-path defect above along the way; graceful shutdown stops the consumer before the database closes.
- Performance: no steady-state change expected — processing path, queue, and thread count are unchanged.
- Developer Experience: the core layer no longer references the API layer;
framework/src/mainbecomes@Lazy-free.
Compatibility
- Breaking Change: No
- Default Behavior Change: No — filter delivery, queue semantics, and shutdown semantics are preserved
- Migration Required: No
Additional Notes
- Do you have ideas regarding implementation? Yes
- Are you willing to implement this feature? Yes
Activity
lxcmyf commentedon Sep 10, 2026
Two lifecycle details seem worth making explicit in this refactor:
Managercurrently startsfilterEsonly whenisJsonRpcFilterEnabled()is true. SinceTronJsonRpcImplis an eager singleton, an unconditional@PostConstructstart would create a consumer thread even when both filter APIs are disabled. The new owner should preserve the same enable gate, andclose()should be tested when the consumer was never started.catch (Throwable)behavior, which also swallowsTronError,VirtualMachineError,ThreadDeath, andLinkageError. Ordinary per-capsule failures can remain isolated, but process-fatal errors should terminate/propagate according to the node fatal-error policy rather than being logged and followed by another loop iteration. A test distinguishing recoverableRuntimeExceptionfrom fatalErrorwould help lock down that boundary.0xbigapple commentedon Sep 10, 2026
@lxcmyf Thanks for the comments.
Yes, we need to add the enable check in the
@PostConstructofTronJsonRpcImpl, so the consumer only starts whenisJsonRpcFilterEnabled()is true.I think the error handling should stay as before. This refactor changes ownership and lifecycle, not error handling, so the loop is moved unchanged. The consumer thread only does in-memory matching on the capsule (the database reads happen on the producer side in
Manager.postLogsFilter), so noTronErrorthrow site is reachable from it, and a swallowed failure only affectseth_getFilterChangesfor that block as before. Handling ofVirtualMachineError/ThreadDeath/LinkageErroris a node-wide question rather than something specific to this loop, so I would rather leave it out of this issue; we can open a separate one if needed.lxcmyf commentedon Sep 10, 2026
Thanks, the enable gate addresses the first concern. I agree that the fatal-error policy can be tracked separately to keep this refactor focused.
One nuance: the absence of an explicit
TronErrorthrow site does not makecatch (Throwable)harmless—VirtualMachineError,ThreadDeath, andLinkageErrorcan still originate from allocation, class loading, or runtime internals in an in-memory path. Please link a follow-up before merge so this behavior remains visible.For this issue, the tests should still verify that interruption terminates the consumer and that repeated
close()is safe in both enabled and disabled modes.0xbigapple commentedon Sep 11, 2026
@lxcmyf Thanks for the reply.
For the fatal-error policy, a separate issue needs a detailed analysis and a concrete proposal first, so I will raise it once that is ready.
For the tests: I will add cases covering interruption of the consumer thread and repeated
close()in both enabled and disabled modes.waynercheung commentedon Sep 11, 2026
+1 on the direction. Explicitly stopping the consumer in
ApplicationImpl.shutdown()beforedbManager.close()fits the existing shutdown orchestration, including the ordering already used forEventService.@0xbigapple Thanks for confirming the lifecycle tests. Three details would help make that coverage precise:
Repeated close reflects the proposed production sequence.
TronJsonRpcImplimplementsCloseable, and Spring 5.3.39 infers itsclose()callback. With the explicit application-level call added, normal shutdown will invoke it again during bean destruction insuper.doClose(). It would be useful to cover that sequence: explicit close with the consumer running, followed by context destruction, plus the disabled case.Graceful stop and interruption need separate tests.
ExecutorService.shutdown()does not interrupt the running consumer task, even when it is waiting in the capsule queue'spoll(). The graceful path therefore needs a reliably visible stop flag. Please cover both an empty-queue consumer stopping without forced cancellation, and interruption duringpoll()while the stop flag is still true, verifying that the loop exits.Include an in-flight parallel matching case.
handleLogsFilter()waits onlogsFilterPool.submit(...).join(), which does not return merely because the consumer is interrupted. A latch-based test could verify that graceful shutdown waits for the current capsule before shutting down the matching pool and clearing caches, without draining queued capsules (setFilterParallelThresholdmakes the parallel branch reachable from a unit test). Please also clarify what happens if shutdown times out or the closing thread is interrupted, sinceshutdownAndAwaitTermination()can return without confirming termination.Happy to review the PR.
lxcmyf commentedon Sep 11, 2026
That works for me. The planned tests cover the lifecycle concerns for this refactor. Please link the fatal-error follow-up back to this issue when it is opened so the remaining behavior is traceable. No further blocker from my side.
0xbigapple commentedon Sep 22, 2026
@waynercheung Thanks. All three will be covered by unit tests in the upcoming PR:
allowCircularReferences(false)); plus repeated close in disabled mode.poll()exits the loop.close()waits for the in-flight capsule before shutting downlogsFilterPool, and queued capsules are not drained.On timeout or an interrupted closer,
close()forcesshutdownNow(), keeps the interrupt status and returns, so shutdown continues. An in-flight capsule may then be dropped with an ERROR log, which is not client-visible since filters do not survive the process.lxcmyf commentedon Sep 24, 2026
This addresses the three lifecycle questions raised above. The graceful-path contract is now clear: finish the in-flight capsule, do not drain queued capsules, then stop the matching pool and clear the caches.
Please make the forced-path test assert both that the closer interrupt status is preserved and that the executors are in the shutdown state before
close()returns. SinceshutdownNow()does not itself guarantee termination, the implementation and logging should not describe termination as confirmed unlessisTerminated()is true. With those assertions, I have no further lifecycle concerns.