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

fix deleteRecord #1425

Merged
merged 1 commit into from
Jul 20, 2019
Merged

fix deleteRecord #1425

merged 1 commit into from
Jul 20, 2019

Conversation

evazca
Copy link
Contributor

@evazca evazca commented Jul 11, 2019

The pr is to fix a bug about delete records. Currently sarama send the request to the controller. It should be sent to the leader according the code in kafka client.

related with #1377

@@ -81,6 +81,28 @@ func (err ConfigurationError) Error() string {
// See https://cwiki.apache.org/confluence/display/KAFKA/A+Guide+To+The+Kafka+Protocol#AGuideToTheKafkaProtocol-ErrorCodes
type KError int16

// MultiError is used to contain multi error.
type MultiError struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason to add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the delete record request is sent to leader by leader. Thus we may receive multi error response and probably the errors are not the same. I use MultiError to contain the array of error.

return err
partitionPerBroker := make(map[*Broker][]int32)
for partition := range partitionOffsets {
broker, err := ca.client.Leader(topic, partition)
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting looks strange, can you please run gofmt again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gofmt again but nothing changed. Can you please tell me what's the format problem. I'll fix it manually.

@bai
Copy link
Contributor

bai commented Jul 17, 2019

Thanks! What @varun06 said; apart from that could you please squash your commits 🙏

@evazca
Copy link
Contributor Author

evazca commented Jul 18, 2019

Thanks! What @varun06 said; apart from that could you please squash your commits 🙏

I have rebased the commit & push again. But I'm not sure whether it is ok to do it right now and shall I squash the commit again after the code review finished.

@varun06 varun06 merged commit 6fce0f9 into IBM:master Jul 20, 2019
d1egoaz pushed a commit that referenced this pull request Jul 22, 2019
#### Version 1.23.1 (2019-07-22)

Bug Fixes:
- Fix fetch delete bug record
  ([1425](#1425)).
- Handle SASL/OAUTHBEARER token rejection
  ([1428](#1428)).
d1egoaz added a commit that referenced this pull request Jul 22, 2019
#### Version 1.23.1 (2019-07-22)

Bug Fixes:
- Fix fetch delete bug record
  ([1425](#1425)).
- Handle SASL/OAUTHBEARER token rejection
  ([1428](#1428)).
@d1egoaz d1egoaz mentioned this pull request Jul 22, 2019
d1egoaz added a commit that referenced this pull request Jul 22, 2019
#### Version 1.23.1 (2019-07-22)

Bug Fixes:
- Fix fetch delete bug record
  ([1425](#1425)).
- Handle SASL/OAUTHBEARER token rejection
  ([1428](#1428)).
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.

3 participants