ctubbsii commented on code in PR #5865:
URL: https://github.com/apache/accumulo/pull/5865#discussion_r2356271227
##########
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:
I'm saying that the problem you are trying to address in this PR, that you
can't use the public API for updating ZK to support this feature in
`accumulo-cluster` when Accumulo is offline, is closely related to the fact
that `ClusterConfigParser` is the wrong tool for the job. It's not just about
future-proofing... it's about the capability being added to the wrong tool...
out of scope of the tool it was added to... and possibly being absent from the
tool where it should exist... whose very existence aligns precisely with what
you're trying to do.
1. ZooPropEditor/Viewer should have the capability to manage this PropKey
type, just as they do for other PropKey types. The whole reason for the
existence of those tools is to do what you're trying to do here: to manage
properties in ZK for different config nodes directly (e.g. when Accumulo is not
running). So, there is no need to add that capability to ClusterConfigParser.
2. It should be removed from ClusterConfigParser, which should have no
knowledge of Accumulo generally, and should not have any dependencies on other
Accumulo code. It has one job and one job only: read YAML and display in a
format usable by our scripts. It shouldn't be doing any work other than reading
the YAML file it was given.
3. `accumulo-cluster` can still support the command line options for
managing RGs, but it's probably not necessary, since it would only be a
passthrough for ZooPropEditor, which is the tool that should be doing the work.
The capability should be removed from ClusterConfigParser entirely, and
ZooPropEditor needs to have the equivalent capability to handle RG property
configs, if it doesn't already.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
