Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use ZooKeeper with a base directory ("chroot") instead of always specifying full path #4809

Open
ctubbsii opened this issue Aug 14, 2024 · 1 comment · May be fixed by #5350
Open

Use ZooKeeper with a base directory ("chroot") instead of always specifying full path #4809

ctubbsii opened this issue Aug 14, 2024 · 1 comment · May be fixed by #5350
Assignees
@meatballspaghetti
Milestone
3.1.0

Comments

@ctubbsii
Copy link
Member

This issue was originally described at https://issues.apache.org/jira/browse/ACCUMULO-4407

We pass around the instanceId a lot in Accumulo, and the vast majority of the time, it's just so we can pass it as part of the path to a ZooKeeper object that is already constructed. We can avoid a lot of this, and simplify the code quite a bit if we just construct the ZooKeeper object using the instanceId as part of the connection string, so all absolute paths are then relative to the instanceId directory, similar to one might do with chroot.

Here's an example of what the code change would look like:

    // the current way
    var directZK = new ZooKeeper(zookeepers, 200, null);
    directZK.getChildren(Constants.ZROOT + "/" + instanceId + Constants.ZNAMESPACES, false)
        .forEach(System.out::println);

    // the better way
    var chrootZK = new ZooKeeper(zookeepers + Constants.ZROOT + "/" + instanceId, 200, null);
    chrootZK.getChildren(Constants.ZNAMESPACES, false).forEach(System.out::println);

We still need to connect without the instanceId when we look up the instanceId from the instance name (which is needed on the client side), but in all other cases, we don't need it.
@ctubbsii ctubbsii added this to the 3.1.0 milestone Aug 14, 2024
@meatballspaghetti
Copy link
Contributor

I can look into this.

@ctubbsii ctubbsii mentioned this issue Nov 27, 2024
Merged
This was referenced Dec 14, 2024
Closed
Merged
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Jan 28, 2025
@meatballspaghetti
Change ZK paths for Chroot compatibility
1af07f3 
- Delete ClientContext.getZooKeeperRoot()
- Remove uses of ZooUtil.getRoot() in which Chroot would apply
- Inline TabletsMetadata.getRootMetadata()
- Remove AdminTest.testZooKeeperTserverPath()
- Remove other functionality whose purpose revolved around the
  root path which will be Chroot-ed.

Resolves: apache#4809
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Jan 30, 2025
@meatballspaghetti
Clean up compilation errors from Chroot changes
86525ae 
- Fix misc errors and refactors from previous ZooKeeper Chroot
  path changes
- Add expect line to PropStoreEventTest temporarily

Resolves: apache#4809
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Feb 6, 2025
@meatballspaghetti
Change ZK paths for Chroot compatibility
a4eda39 
- Delete ClientContext.getZooKeeperRoot()
- Remove uses of ZooUtil.getRoot() in which Chroot would apply
- Inline TabletsMetadata.getRootMetadata()
- Remove AdminTest.testZooKeeperTserverPath()
- Remove other functionality whose purpose revolved around the
  root path which will be Chroot-ed.

Resolves: apache#4809
meatballspaghetti added a commit to meatballspaghetti/accumulo that referenced this issue Feb 6, 2025
@meatballspaghetti
Clean up compilation errors from Chroot changes
8ee2ba2 
- Fix misc errors and refactors from previous ZooKeeper Chroot
  path changes
- Add expect line to PropStoreEventTest temporarily

Resolves: apache#4809
@meatballspaghetti meatballspaghetti linked a pull request Feb 21, 2025 that will close this issue
Open
@ctubbsii ctubbsii linked a pull request Feb 21, 2025 that will close this issue
"Chroot" ZooKeeper to base directory instead of specifying full path #5350
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@meatballspaghetti meatballspaghetti

Labels
None yet
Projects
None yet
Milestone
3.1.0
Development

Successfully merging a pull request may close this issue.

"Chroot" ZooKeeper to base directory instead of specifying full path meatballspaghetti/accumulo
2 participants
@ctubbsii @meatballspaghetti