ctubbsii commented on code in PR #5865:
URL: https://github.com/apache/accumulo/pull/5865#discussion_r2353190851
##########
server/base/src/main/java/org/apache/accumulo/server/conf/cluster/ClusterConfigParser.java:
##########
@@ -159,8 +159,12 @@ private static void validateConfiguredGroups(final
ServerContext ctx, final Set<
if (!zkGroups.contains(cg)) {
if (createMissingRG) {
try {
- ctx.resourceGroupOperations().create(ResourceGroupId.of(cg));
- } catch (AccumuloException | AccumuloSecurityException e) {
+ // cant use API as servers may not be up when this is called
+ // from accumulo-cluster
+ final ResourceGroupId rgid = ResourceGroupId.of(cg);
+ final ResourceGroupPropKey key = ResourceGroupPropKey.of(rgid);
+ key.createZNode(ctx.getZooSession().asReaderWriter());
+ } catch (KeeperException | InterruptedException e) {
Review Comment:
This isn't the best place for this code. It's out of scope of configuration
file parsing, and will make it harder to replace this with `yq` in future.
There's also another place in the main method, shortly after it creates the
ServerContext, where it uses the `context.resourceGroupOperations().list()`
public API.
I think these features to modify ZK in this file should be stripped out, and
instead, it should be added to the existing ZooPropEditor or related admin tool
for managing property configuration nodes in ZK, where it would fit more
naturally alongside the same options for table, namespace, and system configs.
This class should stick to just parsing the configuration file.
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ResourceGroupOperationsImpl.java:
##########
@@ -109,7 +109,7 @@ public Map<String,String> getProperties(ResourceGroupId
group)
TVersionedProperties rgProps = ThriftClientTypes.CLIENT.execute(context,
client ->
client.getVersionedResourceGroupProperties(TraceUtil.traceInfo(),
context.rpcCreds(), group.canonical()),
- ResourceGroupPredicate.exact(group));
+ ResourceGroupPredicate.ANY);
Review Comment:
I think I understand the basis of this change to be:
You can ask any service what the config is for a RG group, because any of
them can query ZK, regardless of what RG they are in.
Is that correct?
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
