Fixes #1225 - Place all table configurations under single Zookeeper path.#1382
Closed
jzgithub1 wants to merge 1 commit intoapache:masterfrom
Closed
Fixes #1225 - Place all table configurations under single Zookeeper path.#1382jzgithub1 wants to merge 1 commit intoapache:masterfrom
jzgithub1 wants to merge 1 commit intoapache:masterfrom
Conversation
…e to prevent erasure from ZooCache and reduce Watchers
ctubbsii
reviewed
Oct 4, 2019
Member
ctubbsii
left a comment
ctubbsii
left a comment
There was a problem hiding this comment.
From what I can see, this change does two things: moves the table configs under a single ZK path, and stops clearing the tableStateCache on certain watched events. I'm not convinced these changes address the problem of too many watchers at all. If it does, I'm very confused as to how. I would also prefer a solution that doesn't move the table config, as I don't think it's necessary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds a new Zookeeper path to put the table configurations for all tables under a single Znode. This path is located at /accumulo/{instance-id}/table_configs in Zookeeper. Many of the other table configurations will be still located under /accumulo/{instance-id}/tables to maintain the current functional operation of the code with the present ZooCache class which has not been modified at all. The relocated configurations are ones like these: table.iterator.majc.vers, table.constraint.1, table.iterator.scan.vers.opt.maxVersions, table.iterator.minc.vers, table.iterator.majc.vers.opt.maxVersions, table.iterator.minc.vers.opt.maxVersions, table.iterator.scan.vers.
Those configuration values were being cleared from the ZooCache during add/remove/clone table operations. This necessitated calling Zookeeper again for all the configurations of all the tables. For large systems that constantly added and removed tables this caused ZooCache.get to created new Watchers on configurations that ZooKeeper already had a watch on. On systems with many TServers with many tables this must have put a larger burden on Zookeeper than we would see in smaller systems.
I think that this pull request reduces the number of Zookeeper Watchers that get created in the ZooCache.get function. I have observed this in trace debugging.
I have run the ingest testing in the Accumulo-Testing project on this code. Also I have verified table creation, deletion and cloning in the accumulo command line interface. I verified the creation of the table configurations at their new locations inside of the ZooKeeper command line interface.