Skip to content

ct-test-srv: cap tracked submissions#8752

Open
mcpherrinm wants to merge 3 commits into
mainfrom
mattm-ct-test-srv-submission-cap
Open

ct-test-srv: cap tracked submissions#8752
mcpherrinm wants to merge 3 commits into
mainfrom
mattm-ct-test-srv-submission-cap

Conversation

@mcpherrinm
Copy link
Copy Markdown
Contributor

When using long-running test environments, ct-test-srv grows until it OOMs, which is causing some noise in my efforts to right-size memory usage.

When using long-running test environments, ct-test-srv grows until it
OOMs, which is causing some noise in my efforts to right-size memory usage.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to bound memory growth in ct-test-srv by capping the number of distinct hostnames tracked for CT submission counts.

Changes:

  • Adds a submissionsCap field and SubmissionsCap config option.
  • Moves submission-count update logic into a helper intended to enforce the cap.
  • Defaults the cap to 100 when unset.
Comments suppressed due to low confidence (2)

test/ct-test-srv/main.go:175

  • The helper indexes the map with hostnames, but that identifier is only local to addChainOrPre and is not in scope here. This causes a compile error and also ignores the hostname argument.
	_, ok := is.submissions[hostnames]
	if ok || len(is.submissions) < is.submissionsCap {
		is.submissions[hostnames]++

test/ct-test-srv/main.go:169

  • After removing the inline increment from addChainOrPre, this new helper is never called anywhere in the package. Even if the compile errors are fixed, successful CT submissions will no longer update is.submissions, so /submissions will always return 0 for new hosts.
func (is *integrationSrv) addSubmission(hostname String) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/ct-test-srv/main.go Outdated
Comment thread test/ct-test-srv/main.go
Comment on lines +224 to +226
cap := p.SubmissionsCap
if cap == 0 {
cap = 100
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread test/ct-test-srv/main.go
Comment on lines +174 to +177
_, ok := is.submissions[hostnames]
if ok || len(is.submissions) < is.submissionsCap {
is.submissions[hostnames]++
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that's the idea. I don't think evicting or reset is useful for integration tests

Comment thread test/ct-test-srv/main.go Outdated
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