You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2024/10/08 19:10:37 UTC
[PR] refactors filtering getServers API call [accumulo]
keith-turner opened a new pull request, #4960:
URL: https://github.com/apache/accumulo/pull/4960
Changes filtering in the getServers API call so that it can prune branches while walking the tree of servers in zookeeper.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactors filtering in getServers API call [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4960:
URL: https://github.com/apache/accumulo/pull/4960#discussion_r1792479373
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java:
##########
@@ -520,43 +520,64 @@ public ServerId getServer(ServerId.Type type, String resourceGroup, String host,
@Override
public Set<ServerId> getServers(ServerId.Type type) {
- return getServers(type, null);
+ AddressPredicate addressPredicate = addr -> true;
+ return getServers(type, rg -> true, addressPredicate);
}
@Override
- public Set<ServerId> getServers(ServerId.Type type, Predicate<ServerId> test) {
+ public Set<ServerId> getServers(ServerId.Type type, Predicate<String> resourceGroupPredicate,
+ BiPredicate<String,Integer> hostPortPredicate) {
+ Objects.requireNonNull(type, "Server type was null");
+ Objects.requireNonNull(resourceGroupPredicate, "Resource group predicate was null");
+ Objects.requireNonNull(hostPortPredicate, "Host port predicate was null");
+
+ AddressPredicate addressPredicate = addr -> {
+ var hp = HostAndPort.fromString(addr);
+ return hostPortPredicate.test(hp.getHost(), hp.getPort());
+ };
+
+ return getServers(type, resourceGroupPredicate, addressPredicate);
+ }
+
+ private Set<ServerId> getServers(ServerId.Type type, Predicate<String> resourceGroupPredicate,
+ AddressPredicate addressPredicate) {
+
final Set<ServerId> results = new HashSet<>();
+
switch (type) {
case COMPACTOR:
- context.getServerPaths().getCompactor(rg -> true, addr -> true, true)
+ context.getServerPaths().getCompactor(resourceGroupPredicate::test, addressPredicate, true)
.forEach(c -> results.add(createServerId(type, c)));
break;
case MANAGER:
ServiceLockPath m = context.getServerPaths().getManager(true);
- Optional<ServiceLockData> sld = context.getZooCache().getLockData(m);
- String location = null;
- if (sld.isPresent()) {
- location = sld.orElseThrow().getAddressString(ThriftService.MANAGER);
- HostAndPort hp = HostAndPort.fromString(location);
- results.add(new ServerId(type, Constants.DEFAULT_RESOURCE_GROUP_NAME, hp.getHost(),
- hp.getPort()));
+ if (m != null) {
Review Comment:
Could be the Manager is not fully up yet. Probably safe to leave it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactors filtering in getServers API call [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #4960:
URL: https://github.com/apache/accumulo/pull/4960
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactors filtering in getServers API call [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4960:
URL: https://github.com/apache/accumulo/pull/4960#discussion_r1792456009
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java:
##########
@@ -520,43 +520,64 @@ public ServerId getServer(ServerId.Type type, String resourceGroup, String host,
@Override
public Set<ServerId> getServers(ServerId.Type type) {
- return getServers(type, null);
+ AddressPredicate addressPredicate = addr -> true;
+ return getServers(type, rg -> true, addressPredicate);
}
@Override
- public Set<ServerId> getServers(ServerId.Type type, Predicate<ServerId> test) {
+ public Set<ServerId> getServers(ServerId.Type type, Predicate<String> resourceGroupPredicate,
+ BiPredicate<String,Integer> hostPortPredicate) {
+ Objects.requireNonNull(type, "Server type was null");
+ Objects.requireNonNull(resourceGroupPredicate, "Resource group predicate was null");
+ Objects.requireNonNull(hostPortPredicate, "Host port predicate was null");
+
+ AddressPredicate addressPredicate = addr -> {
+ var hp = HostAndPort.fromString(addr);
+ return hostPortPredicate.test(hp.getHost(), hp.getPort());
+ };
+
+ return getServers(type, resourceGroupPredicate, addressPredicate);
+ }
+
+ private Set<ServerId> getServers(ServerId.Type type, Predicate<String> resourceGroupPredicate,
+ AddressPredicate addressPredicate) {
+
final Set<ServerId> results = new HashSet<>();
+
switch (type) {
case COMPACTOR:
- context.getServerPaths().getCompactor(rg -> true, addr -> true, true)
+ context.getServerPaths().getCompactor(resourceGroupPredicate::test, addressPredicate, true)
.forEach(c -> results.add(createServerId(type, c)));
break;
case MANAGER:
ServiceLockPath m = context.getServerPaths().getManager(true);
- Optional<ServiceLockData> sld = context.getZooCache().getLockData(m);
- String location = null;
- if (sld.isPresent()) {
- location = sld.orElseThrow().getAddressString(ThriftService.MANAGER);
- HostAndPort hp = HostAndPort.fromString(location);
- results.add(new ServerId(type, Constants.DEFAULT_RESOURCE_GROUP_NAME, hp.getHost(),
- hp.getPort()));
+ if (m != null) {
Review Comment:
This line is fixing a bug I am seeing when running test. When m is null on the next line an NPE will happen w/o this check
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactors filtering in getServers API call [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4960:
URL: https://github.com/apache/accumulo/pull/4960#issuecomment-2402069733
Does this close #4961?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org