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

Remove the old log buffer #13821

Merged
merged 8 commits into from
Jul 19, 2024
Merged

Remove the old log buffer #13821

merged 8 commits into from
Jul 19, 2024

Conversation

ferglor
Copy link
Collaborator

@ferglor ferglor commented Jul 11, 2024

https://smartcontract-it.atlassian.net/browse/AUTO-9355

This PR removes the old log buffer and the corresponding feature flag from the code; we want to use the new buffer by default moving forward

Test results

  • Not every block contains logs for each upkeep - this seems to be because we have no control over exactly what logs get emitted and when, as this is up to the geth node
  • It seems that some blocks don't contain any logs for certain upkeep, while other blocks contain a number of logs per upkeep, e.g block 1010 and 1011 don't have any logs for upkeep 0, but block 1007 had 5 logs:
"0":{
      "1000":2,
      "1001":2,
      "1002":3,
      "1003":1,
      "1004":1,
      "1005":2,
      "1006":2,
      "1007":5,
      "1008":1,
      "1009":2,
      "1012":1,
      ...
  • This PR was created to help gather insights to exactly how many logs were dequeued per upkeep per block
  • We were able to measure log availability and the number of logs dequeued, as well as instances of logs missing from certain block ranges, and tracking cleaned/dropped logs
  • As an example of logs dequeued, for one upkeep we can see how many logs were dequeued in a selected block range:
{
	"104500045091363938937248279319830213588488308612973768466030697520485050708437": {
		"1902": [
			2
		],
		"1903": [
			2
		],
		"1904": [
			2
		],
		"1905": [
			2
		],
		"1906": [
			2
		],
		"1907": [
			2
		],
		"1908": [
			2
		],
		"1909": [
			2
		],
		"1910": [
			2
		],
		"1938": [
			2
		],
...
  • This shows us that 1902-1910 were dequeued from once, and each dequeue was for 2 logs (minimum guaranteed)
  • The gap in dequeued blocks between 1910 and 1938 is further evidenced by a lack of available logs in that block range, e.g. a different dataset focuses on log availability per upkeep per block shows no logs were available in that block range:
		"1910": [
			6
		],
		"1938": [
			4
		],
  • This shows that for this upkeep, block 1910 had 6 logs available for dequeuing, followed by a gap of availability until block 1938, which had 4 logs available for dequeuing

50x2 upkeeps

  Old buffer New buffer
Average 48.403181 45.50054
Median 39 36
90th Percentile 100 93
99th Percentile 152 132
Max 283 280
Perform Count Fast Execution: 321176 334311
Total Expected Log Triggering Events: 360000 360000
Total Log Triggering Events Emitted: 360200 360200
Total Events Missed: 706 340
Percent Missed: 0.196002 0.094392

50x10 upkeeps

  Old buffer New buffer
Average 130.243086 125.347883
Median 126 117
90th Percentile 241 235
99th Percentile 270 276
Max 278 284
Perform Count Fast Execution: 114552 211442
Total Expected Log Triggering Events: 1800000 1800000
Total Log Triggering Events Emitted: 1801000 1801000
Total Events Missed: 1667092 1537481
Percent Missed: 92.564797 85.368184

50x10+1 upkeeps

  Old buffer New buffer
Average 128.517138 94.876548
Median 122 74
90th Percentile 239 204
99th Percentile 269 268
Max 276 288
Perform Count Fast Execution: 109814 149013
Total Expected Log Triggering Events: 1800000 1800000
Total Log Triggering Events Emitted: 1801000 1801000
Total Events Missed: 1672352 1585082
Percent Missed: 92.856857 88.011216

Node upgrade tests passing: https://github.com/smartcontractkit/chainlink/actions/runs/10010662768/job/27672458705

@ferglor ferglor requested review from a team as code owners July 11, 2024 13:38
@ferglor ferglor requested a review from a team as a code owner July 11, 2024 16:43
krehermann
krehermann previously approved these changes Jul 17, 2024
@@ -538,13 +503,7 @@ func (p *logEventProvider) readLogs(ctx context.Context, latest int64, filters [

func (p *logEventProvider) syncBufferFilters() error {
p.lock.RLock()
buffVersion := p.opts.BufferVersion
p.lock.RUnlock()
defer p.lock.RUnlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

we can probably remove the Lock/unlock completely here since we dont need to read buffer version

@cl-sonarqube-production
Copy link

@ferglor ferglor enabled auto-merge July 19, 2024 15:34
@ferglor ferglor added this pull request to the merge queue Jul 19, 2024
Merged via the queue into develop with commit 5b668c1 Jul 19, 2024
126 checks passed
@ferglor ferglor deleted the feature/AUTO-9355-3 branch July 19, 2024 16:49
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.

4 participants