Conversation
Contributor
📊 API Diff Results
|
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.
cre-5665
ref. full code fix and test is in PR: #2284
That's a regression test that presents a blocking within the durable emitter and how it breaks 2.54/2.55 vs 2.56/2.57.
Test passes. It clearly demonstrates the regression:
4113x slower
Investigation
The
DurableEmitterEnabledconfig default was changed fromfalsetotruein core node v2.56.0-rc.0. This causes every workflow metric/telemetry emission to block on a synchronous Postgres INSERT through the durable emitter's insert coalescer, adding ~100ms+ per emission event to workflow execution time.The following commits between v2.55.0-rc.0 and v2.56.0-rc.0 contributed to the regression:
4e99fa3e76- Durable Emitter: Load Testing Updates (#23001): Added explicit batch/timeout/retransmit config toapplication.go. Changed durable emitter from default config to custom config withInsertBatchSize=500,InsertBatchFlushInterval=100ms,BatchSize=1000,MaxPublishTimeout=10s,MessageBufferSize=50000.302b792a3a- Durable Emitter: TOML Configs (#23096): MadeRetransmitBatchSizeandEventTTLconfigurable via TOML. The default change forDurableEmitterEnabledfromfalsetotruewas introduced in this commit's changes toconfig_telemetry.go.51cfafafd9- Durable emitter- add max queue payload TOML configuration (#23081): AddedMaxQueuePayloadBytesconfig (default 1 GiB).chainlink-common bump (
9264d4444ce0->4033d0253977): Includes3823ee79(DurableEmitter Optimizations - mark-to-delete, fallback client removal) and712bad3e(Resource Managers with durable metering for CRE).Fix Options
Immediate Mitigation
Set
DurableEmitterEnabled = falsein the node's TOML config under[Telemetry]:This restores the v2.55 behavior where
GlobalEmitreturnsErrNotInitializedimmediately.Proper Fix
Make
durableemitter.GlobalEmit()non-blocking. The insert coalescer should queue the event and return immediately without waiting for the INSERT to complete. The current blocking behavior ("block until the multi-row INSERT completes") is inappropriate for the hot path of workflow metric emission.Alternative Fix
Decouple the durable emitter from the workflow emission path. Instead of calling
durableemitter.GlobalEmit()synchronously inemitRawMessage(), emit to the durable emitter asynchronously (e.g., via a goroutine or non-blocking channel send).