perf(fabricx/finality): batch finality via shared poller#1849
Open
EvanYan1024 wants to merge 2 commits into
Open
perf(fabricx/finality): batch finality via shared poller#1849EvanYan1024 wants to merge 2 commits into
EvanYan1024 wants to merge 2 commits into
Conversation
a33db8e to
4ac1b86
Compare
Replace the per-transaction remote status check (a speculative GetTransactionStatus at registration time plus a per-tx fallback, which serialized every queue worker on its own committer round-trip) with a single shared poller. It sweeps all pending txs each interval, batches the committer status query (and the token-request-hash lookup for valid txs), and hands terminal txs to the existing worker pool for notification with no network I/O on that path. The poller uses the query service's batched GetTransactionStatuses when available (added to the FabricX query service in hyperledger-labs/fabric-smart-client#1553) and falls back to per-tx GetTransactionStatus otherwise, so no FSC version bump is required. In the per-tx path a failing query only skips that tx, since a not-yet-committed tx returns an error rather than a status. A tx can have several finality waiters; the pending set keeps a waiter list per txID and notifies all of them on resolution. Poll interval, batch size, and pending TTL are read from configuration (token.finality.poller.*), following the existing notification queue and lookup config patterns. Signed-off-by: Evan <evanyan@sign.global>
4ac1b86 to
f4396a2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1848.
Motivation
NSListenerManagerresolves FabricX finality with one remote committer query per transaction: a speculativeGetTransactionStatusat registration (almost always seeing a not-yet-committed tx) plus per-tx work items that each block a queue worker on its own network round-trip. In our deployment (Fabric-X backend, sustained submission from 4000 concurrent clients), finality resolution settled ~92 tx/s while the chain committed ~1400 tx/s, accumulating >100k inflight finality watchers.Changes
AddFinalityListenernow adds the tx to a pending set instead of enqueuing a per-txTxCheck; a single shared poller, started lazily on first registration, sweeps the pending set every poll interval.GetTransactionStatuses(perf(fabricx/queryservice): add batched GetTransactionStatuses hyperledger-labs/fabric-smart-client#1553) when the query service provides it, and falls back to per-txGetTransactionStatusotherwise. No FSC dependency bump is required; the batch path activates once the FSC dependency includes Add benchmarks to track performance over time #1553. In the per-tx path a failing query only skips that tx (a not-yet-committed tx returns an error rather than a status), so one fresh tx cannot fail the whole chunk.GetStatesquery, grouped by namespace.token.finality.poller.interval/.batchSize/.pendingTTL), following the existingtoken.finality.notification.*andtoken.fabricx.lookup.*config patterns; unset values fall back to defaults (1s / 2000 / 10m).docs/configuration.md: documented the newtoken.finality.poller.*keys and updated the now-stale note about the immediate status query at subscription time.Results
With this change our deployment's finality resolution improved ~5x immediately and stopped being network-bound (remaining limits in our runs were local, e.g. DB connection pool sizing). It has been running in our test environments under sustained load since.
Notes for reviewers
ListenerManageris still constructed and held, but no longer used for status delivery. Happy to either drop it or re-wire it so push notifications complement the poller (poller as sweep/fallback) if that's the preferred direction.