Skip to content

refactor(framework): decouple Manager from TronJsonRpcImpl - #6990

Open
0xbigapple wants to merge 5 commits into
tronprotocol:release_v4.8.3from
0xbigapple:refactor/decouple-jsonrpc-filter
Open

0xbigapple wants to merge 5 commits into
tronprotocol:release_v4.8.3from
0xbigapple:refactor/decouple-jsonrpc-filter

Conversation

@0xbigapple

Copy link
Copy Markdown
Collaborator

What does this PR do?
close #6963
Removes the Manager → TronJsonRpcImpl reverse dependency left by #6732, where core-layer Manager holds the json-rpc filter consumer through a @Lazy injection.

  • Adds a standalone FilterCapsuleQueue bean; Manager.postBlockFilter/postLogsFilter produce into it, so Manager no longer references anything under org.tron.core.services.jsonrpc (repo-wide framework/src/main is now @Lazy-free).
  • Moves the consumer loop into TronJsonRpcImpl: started by @PostConstruct when isJsonRpcFilterEnabled(), single daemon thread, stopped by close().
  • TronJsonRpcImpl constructor becomes (NodeInfoService, Wallet, Manager); setManager() is removed and the direct-construction test sites are migrated.
  • close() is rewritten: an AtomicBoolean guard makes it idempotent and serves as a visibility-safe stop flag, the loop exits on interrupt, and the consumer is awaited before logsFilterPool shuts down so an in-flight capsule completes on graceful shutdown.
  • ApplicationImpl.shutdown() now closes the consumer after producers stop and before dbManager.close(); the later Spring-destruction close() is a no-op.
  • The instanceof dispatch logs a warning for unknown FilterTriggerCapsule subtypes instead of silently dropping them.

Runtime behavior of the filter API is unchanged: same unbounded queue, same discard-on-shutdown semantics, one consumer shared by the FullNode/solidity/PBFT json-rpc services.

Why are these changes required?

Follow-up agreed in the #6732 review (see discussion). FullNode sets allowCircularReferences(false); @Lazy, like ObjectProvider or a runtime getBean(), only makes Spring tolerate the cycle. The reverse core → API edge stays, obscuring the component graph, and later json-rpc cleanups keep copying the pattern. This PR removes the edge instead.

This PR has been tested by:

  • Unit Tests
  • Manual Testing
    • private chain: block production with registered filters; eth_getFilterChanges delivers; SIGTERM log order confirms consumer → logs-filter-pool → dbManager → context close, single shutdown episode, no errors
    • Nile and mainnet lite nodes: 5 restart cycles each with 100 filters created and polled per cycle (~2,500 queries/cycle, zero rpc errors); on mainnet the log path delivered 20k+ real contract log entries through the new queue; filters correctly report filter not found after restart; no RejectedExecutionException, no bean-cycle errors, shutdown order identical in every cycle

Follow up

Extra details

}
// The consumer loop submits to logsFilterPool (over-threshold path), so it must
// terminate before the pool shuts down.
ExecutorServiceManager.shutdownAndAwaitTermination(filterEs, filterEsName);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD] shutdownAndAwaitTermination() does not guarantee that the consumer has terminated: if the closing thread is interrupted, it calls shutdownNow() and returns without waiting again. We then shut down logsFilterPool while the consumer may still submit work to it.

I reproduced close() returning with filterEs.isTerminated() == false and logsFilterPool.isShutdown() == true; resuming the consumer then caused a RejectedExecutionException.

Could we preserve the consumer-before-pool shutdown ordering on this interruption path and add a regression test? The existing interrupted-close test registers no filters, so it never exercises submission to logsFilterPool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[Feature] decouple json-rpc filter processing from Manager

3 participants