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

Patches #662

Merged
merged 12 commits into from
Jan 21, 2024
Merged

Patches #662

merged 12 commits into from
Jan 21, 2024

Conversation

twmb
Copy link
Owner

@twmb twmb commented Jan 21, 2024

No description provided.

twmb added 11 commits January 21, 2024 08:55
See embedded comment. Not having this logic was making it look like
issuing a request failed with something like NOT_LEADER_FOR_PARTITION
(if there were enough retries) -- this is not a failed request, and
_some_ of the response may be successful.
When I originally wrote this I was thinking I could use one struct for
the request and response, but I changed that because I noticed the
response names a field differently absolutely no reason. I forgot to
remove Subject and Err in the SetCompatibility struct.
This takes all goroutines out of client initialization sub functions (ie
not the explicit ones _in_ client initialization) and avoids starting a
goroutine if we never even make it to the manage loop anyway (which only
happens once we finally fetch metadata)

Also skips autocommitting if there is nothing to commit. This is the
same behavior as previously, but now we have a log that explicitly says
the commit is being skipped.
…titions"

This restriction existed originally before AddConsumeTopics came into
being. Now that AddConsumeTopics exists, we can just preemptively seed
the client with all topics we are setting the offsets of, and then set
the offsets.
VersionStrings and FromString will make it easy to write tests that
iterate over all known broker versions.
If a topic has a load error, findNewAssignments will return new
assignments for topics that have no partitions. This emits extraneous
log lines and looks like progress is being made when it isn't,
A consumer should be notified when it cannot make progress due to
metadata load errors. We now
* unconditionally inject a fake fetch if a load error is not retryable
* inject a fake fetch if the user has configured KeepRetryableFetchErrors
Failing in CI without unlimited retries (i.e. slow cluster).
@twmb twmb merged commit a8e33ff into master Jan 21, 2024
6 checks passed
@twmb twmb deleted the patches branch January 21, 2024 20:06
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