Skip to content

DONOTMERGE#14183

Open
sushanb wants to merge 1 commit intogoogleapis:mainfrom
sushanb:aa_b
Open

DONOTMERGE#14183
sushanb wants to merge 1 commit intogoogleapis:mainfrom
sushanb:aa_b

Conversation

@sushanb
Copy link
Copy Markdown
Contributor

@sushanb sushanb commented Mar 16, 2026

No description provided.

@sushanb sushanb requested review from a team as code owners March 16, 2026 19:29
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new function, CallSingleChannel, for what appears to be testing purposes, given the 'DONOTMERGE' title. The function establishes a gRPC connection and continuously sends requests. My review identifies a couple of areas for improvement: a misleading comment and a missing backoff mechanism in an error handling path, which could lead to high CPU usage.

Comment on lines +172 to +173
log.Printf("[Worker %d] Prime() failed: %v", workerID, err)
// Tiny backoff to prevent CPU thrashing if the connection fully drops
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The comment on line 173 indicates an intention to add a backoff, but it's not implemented. Without a backoff, the loop could spin and consume high CPU if Prime() fails repeatedly. This suggestion adds a small time.Sleep() to implement the backoff as intended.

Suggested change
log.Printf("[Worker %d] Prime() failed: %v", workerID, err)
// Tiny backoff to prevent CPU thrashing if the connection fully drops
log.Printf("[Worker %d] Prime() failed: %v", workerID, err)
time.Sleep(100 * time.Millisecond) // Tiny backoff to prevent CPU thrashing if the connection fully drops

}

// CallSingleChannel connects to Bigtable using the DirectPath C2P scheme,
// and continuously maintains 200 in-flight Prime requests on the same channel
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The function comment hardcodes '200' as the number of in-flight requests. However, this is determined by the requestsInFlight parameter. The comment should be updated to reflect that this value is configurable to avoid confusion.

Suggested change
// and continuously maintains 200 in-flight Prime requests on the same channel
// and continuously maintains a configurable number of in-flight Prime requests on the same channel

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.

1 participant