You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/03/01 12:30:59 UTC
[GitHub] [bookkeeper] mkozioro opened a new pull request #2632: Fixed doGetLeaves method returning HashSet containing null
mkozioro opened a new pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632
Descriptions of the changes in this PR:
doGetLeaves(non_existing_scope) will return empty HashSet instead of HashSet with null element.
### Motivation
I've found my Apache Pulsar cluster polluted with log:
```
12:01:46.462 pulsar2-dev pulsar-broker {"logLevel":"ERROR","logThread":"pulsar-io-23-1","logger":"org.apache.bookkeeper.client.RackawareEnsemblePlacementPolicyImpl","message":"found non-BookieNode: null as leaf of defaultrack: /default-rack","stack_trace":null}
```
After digging I've found that, because https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java#L324
was returning HashSet with null node, when no bookkeepers were in /default-rack.
### Changes
- Modified doGetLeaves() method to return empty HashSet instead of HashSet with null element when scope node can't be found.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli merged pull request #2632: Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #2632: Fixed doGetLeaves method returning HashSet containing null
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632#issuecomment-787917052
@mkozioro thanks for contributing your fix
we can pick it to active release branches, are you interested in a particular version? 4.13 ?
can you please add a unit test that reproduces the problem ?
also the title of the PR is not telling about the impact of the patch "doGetLeaves" is not very representative.
Probably you can change it and refer to RackawareEnsemblePlacementPolicyImpl and/or reducing spammy log
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] mkozioro commented on pull request #2632: Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Posted by GitBox <gi...@apache.org>.
mkozioro commented on pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632#issuecomment-787942401
Hi @eolivelli,
I've added test for failing case.
I'm interested in 4.12 branch.
Is PR name little better now?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] mkozioro commented on pull request #2632: Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Posted by GitBox <gi...@apache.org>.
mkozioro commented on pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632#issuecomment-790488688
Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #2632: Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632#issuecomment-787955190
@mkozioro very good !
Can you also add some positive tests ? some test that does not return an empty set ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] mkozioro commented on pull request #2632: Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Posted by GitBox <gi...@apache.org>.
mkozioro commented on pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632#issuecomment-787998041
Hi @eolivelli,
I've added test case for getLeaves() positive case and also for excluded scope.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #2632: Fix NetworkTopologyImpl#getLeaves returning set with null value in case of non existing scope
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2632:
URL: https://github.com/apache/bookkeeper/pull/2632#issuecomment-790444707
Pushed to master, branch-4.13 and branch-4.12
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org