> On July 2, 2014, 2:24 p.m., Sean Busbey wrote: > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java, > > lines 69-70 > > <https://reviews.apache.org/r/22685/diff/2/?file=616145#file616145line69> > > > > Is there any problem with these being long lived, e.g. in case a > > particular table is deleted and then recreated with different configuration? > > Bill Havanki wrote: > I don't believe so. Here's some background info. > > - The ZooCachePropertyAccessor object is really just a wrapper around a > ZooCache and contains the logic for finding namespace-specific or > table-specific ZK data in the ZooCache. So the key is whether the ZooCache > stays valid. > - A watcher is set when the ZooCache is initially created. The watcher > pays attention to changes under the ZK node that holds configuration data for > the table. I expect that if a table is deleted, then its configuration data > is removed from ZK and so the watcher will be notified and update the cache. > Alternatively, if a table is re-created, then the data would be flushed > and/or reinitialized, and again the watcher would find out. So I'm relying on > the ZooCache to work correctly.
LGTM - Sean ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22685/#review47187 ----------------------------------------------------------- On June 27, 2014, 5:11 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22685/ > ----------------------------------------------------------- > > (Updated June 27, 2014, 5:11 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2615 > https://issues.apache.org/jira/browse/ACCUMULO-2615 > > > Repository: accumulo > > > Description > ------- > > What started out as a simplification effort on server configurations ended up > with results that aren't so much simpler but are hopefully more > straightforward. Here is a summary of the more important changes. > > * ServerConfiguration is deprecated in favor of the new > ServerConfigurationFactory. > * ServerConfigurationFactory caches configurations not just but namespace or > table ID, but also by instance ID. > * ZooConfigurationFactory takes over the creation logic from > ZooConfiguration. A ZooConfiguration instance is no longer a singleton, but > an ordinary instance. ZooConfigurationFactory caches created ones by instance > ID. > * NamespaceConfiguration and TableConfiguration each keep a static map of > ZooCache instances, rather than a single static instance. The map is keyed by > instance ID and namespace/table (resp.) ID. > * getParentConfiguration() was added to all appropriate configurations. (I > did have an interface declaring the method but decided it was not useful.) > * The logic for retrieving properties from ZooCache instances is refactored > to a new ZooCachePropertyAccessor object. > * NamespaceConfWatcher and TableConfWatcher include the configurations they > watch as fields, so they no longer need to repeatedly look them up (formerly > in ServerConfiguration). As a consequence, the watchers no longer attempt to > handle received WatchedEvents for a different namespace or table. (This > *should* never happen - a warning is logged if it does.) > > > Diffs > ----- > > > server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfWatcher.java > 50ab27ee8dcab41d89789ea409f5e554c56163d3 > > server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfiguration.java > ebb098b01798bd4478f7d487909409bd2ca2baf1 > > server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfiguration.java > 2115f3f28557f1a56b4e117a5ce4e0be920a09d7 > > server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfWatcher.java > 3c24ec49aa4895283e6556b02ed3479dda560b6b > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java > b538d2ecc16a0c4c0456d491afb5e116ae0000b5 > > server/base/src/main/java/org/apache/accumulo/server/conf/TableParentConfiguration.java > 34eb781a5b2807e3457a5f6ac501088ac2fc2e7a > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooCachePropertyAccessor.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java > 696873bb013946c670c9d781cdf43bb0cb143b98 > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java > PRE-CREATION > > server/base/src/test/java/org/apache/accumulo/server/conf/NamespaceConfigurationTest.java > PRE-CREATION > > server/base/src/test/java/org/apache/accumulo/server/conf/ServerConfigurationFactoryTest.java > PRE-CREATION > > server/base/src/test/java/org/apache/accumulo/server/conf/TableConfigurationTest.java > ae73ba69a87bf40de9ec8c3b8f804bb475a810ef > > server/base/src/test/java/org/apache/accumulo/server/conf/ZooCachePropertyAccessorTest.java > PRE-CREATION > > server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/22685/diff/ > > > Testing > ------- > > Unit tests pass. Basic exercising on a one-node cluster (continuous ingest) > successful. All integration tests pass (after ACCUMULO-2951 and related). > > > Thanks, > > Bill Havanki > >
