Skip to content

MINOR: Log member id of the leader when assignment are received#10817

Merged
dajac merged 1 commit into
apache:trunkfrom
dajac:minor-group-coordinator-logs
Jun 7, 2021
Merged

MINOR: Log member id of the leader when assignment are received#10817
dajac merged 1 commit into
apache:trunkfrom
dajac:minor-group-coordinator-logs

Conversation

@dajac

@dajac dajac commented Jun 4, 2021

Copy link
Copy Markdown
Member

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dajac dajac requested a review from hachikuji June 4, 2021 07:21

@hachikuji hachikuji left a comment

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.

LGTM

@showuon showuon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Left a minor comment. You can decide if you want to update it or not. Thanks.

// if this is the leader, then we can attempt to persist state and transition to stable
if (group.isLeader(memberId)) {
info(s"Assignment received from leader for group ${group.groupId} for generation ${group.generationId}. " +
info(s"Assignment received from leader $memberId for group ${group.groupId} for generation ${group.generationId}. " +

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Should we wrap the variable with single/double quote to make it clear? Ex:
info(s"Assignment received from leader '$memberId' for group '${group.groupId}' for generation '${group.generationId'}.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@showuon At the moment, none of the other log in the group coordinator does this so it feel a bit weird to do it only in this case. I prefer keeping it as it is for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No problem. Thank you. :)

@dajac dajac merged commit 1ba217d into apache:trunk Jun 7, 2021
@dajac dajac deleted the minor-group-coordinator-logs branch June 7, 2021 06:34
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