Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize LLO transmitter queue deletes using batching #16166

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

samsondav
Copy link
Collaborator

@samsondav samsondav commented Jan 31, 2025

- This reduces required number of DB connections and should reduce
  overall DB transaction load
- The server doesn't need to be aware of DB deletion at all, the
  persistence manager now handles all of it

@samsondav samsondav requested review from a team as code owners January 31, 2025 12:48
Copy link
Contributor

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

🎖️ No JIRA issue number found in: PR title, commit message, or branch name. Please include the issue ID in one of these.

ro-tex
ro-tex previously approved these changes Jan 31, 2025
@samsondav samsondav enabled auto-merge January 31, 2025 13:38
@samsondav samsondav changed the title Initial set to populate gauge metrics Optimize LLO transmitter queue deletes using batching Jan 31, 2025
@samsondav samsondav force-pushed the set_initial_metrics branch from 32eba2d to 522de6a Compare January 31, 2025 15:52
@samsondav samsondav force-pushed the set_initial_metrics branch 5 times, most recently from 2a8ecb0 to 0f4bb4e Compare January 31, 2025 16:07
- This reduces required number of DB connections and should reduce
  overall DB transaction load
- The server doesn't need to be aware of DB deletion at all, the
  persistence manager now handles all of it
@samsondav samsondav force-pushed the set_initial_metrics branch from 0f4bb4e to 80ca5cf Compare January 31, 2025 16:24
Copy link
Contributor

AER Report: CI Core

aer_workflow , commit , Detect Changes , Clean Go Tidy & Generate , Scheduled Run Frequency , Core Tests (go_core_tests) , GolangCI Lint (.) , Core Tests (go_core_tests_integration) , test-scripts , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , Core Tests (go_core_race_tests) , lint , SonarQube Scan

1. Linting error: defer inside loops

Job ID: Golang Lint (.)

Source of Error:
core/services/llo/mercurytransmitter/persistence_manager.go:128:4: defer: prefer not to defer inside loops (revive)
			defer cancel()
			^

Why: The revive linter detected a defer statement inside a loop, which is generally discouraged because it can lead to resource leaks or unexpected behavior.

Suggested fix: Move the defer cancel() statement outside the loop or ensure it is only called once per loop iteration.

2. Test failure: Condition never satisfied

Job ID: Run tests

Source of Error:
testutils.go:341: 
	Error Trace:	/home/runner/work/chainlink/chainlink/core/internal/testutils/testutils.go:341
	 				/home/runner/work/chainlink/chainlink/core/internal/testutils/testutils.go:365
	 				/home/runner/work/chainlink/chainlink/core/services/llo/mercurytransmitter/persistence_manager_test.go:70
	Error: 	Condition never satisfied
	Test: 	TestPersistenceManagerAsyncDelete
logger.go:146: 2025-01-31T16:54:52.252Z	ERROR	LLOPersistenceManager	mercurytransmitter/persistence_manager.go:194	Failed to prune transmit requests table	{"version": "unset@unset", "serverURL": "wss://example.com/mercury", "err": "llo orm: failed to prune transmissions: context canceled"}
github.com/smartcontractkit/chainlink/v2/core/services/llo/mercurytransmitter.(*persistenceManager).runPruneLoop.func1
	/home/runner/work/chainlink/chainlink/core/services/llo/mercurytransmitter/persistence_manager.go:194
github.com/smartcontractkit/chainlink/v2/core/services/llo/mercurytransmitter.(*persistenceManager).runPruneLoop
	/home/runner/work/chainlink/chainlink/core/services/llo/mercurytransmitter/persistence_manager.go:198
FAIL

Why: The test TestPersistenceManagerAsyncDelete failed because a condition was never satisfied, likely due to the context being canceled before the prune operation could complete.

Suggested fix: Ensure that the context is not prematurely canceled or adjust the test to handle the context cancellation appropriately. Consider increasing the timeout or refactoring the code to ensure the prune operation completes within the expected time frame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants