Skip to content

Commit

Permalink
client: avoid issuing requests to shards that we know are erroring
Browse files Browse the repository at this point in the history
For sharded requests, we often need to load the metadata to perform a
lookup and then issue a request that will ultimately end up failing.
Previously, the assumption was that this would be fine: we would issue a
request, it would fail with a retriable error (NOT_LEADER_FOR_PARTITION
or something), and then we would re-shard, at which time we could hope
that the metadata would load the partition successfully.

Well, that wastes time and leads to unclear error messages

The new behavior is to return an error shard with a portion of the
request that we are not issuing, and then avoid looping at all.

This new behavior uses a bunch of reflect magic to simplify actual
usage. Most of the sharding functions are pretty much copy&paste, so
simplifying things with reflect up top allows us to copy&paste less.
Only one function cannot use our new reflect magic helper, the describe
log dirs sharder.
  • Loading branch information
twmb committed Jun 12, 2021
1 parent 96cb1c2 commit 8b7b43e
Show file tree
Hide file tree
Showing 2 changed files with 240 additions and 85 deletions.
8 changes: 4 additions & 4 deletions pkg/kgo/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ type promisedResp struct {
readEnqueue time.Time
}

var unknownMetadata = BrokerMetadata{
NodeID: -1,
}

// BrokerMetadata is metadata for a broker.
//
// This struct mirrors kmsg.MetadataResponseBroker.
Expand Down Expand Up @@ -131,6 +127,10 @@ type broker struct {

const unknownControllerID = -1

var unknownBrokerMetadata = BrokerMetadata{
NodeID: -1,
}

// broker IDs are all positive, but Kafka uses -1 to signify unknown
// controllers. To avoid issues where a client broker ID map knows of
// a -1 ID controller, we start unknown seeds at MinInt32.
Expand Down
Loading

0 comments on commit 8b7b43e

Please sign in to comment.