----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17860/#review34124 -----------------------------------------------------------
server/src/main/java/org/apache/accumulo/server/monitor/Monitor.java <https://reviews.apache.org/r/17860/#comment64101> Have you manually tested upgrade? It may be better to put this w/ the zookeeper upgrade code in Master. Would make it easier when someones working on 1.7 upgrade to be aware of it and possibly remove it. server/src/main/java/org/apache/accumulo/server/monitor/Monitor.java <https://reviews.apache.org/r/17860/#comment64099> Should this code be shared w/ master instead of duplicating? server/src/main/java/org/apache/accumulo/server/watcher/MonitorLog4jWatcher.java <https://reviews.apache.org/r/17860/#comment64110> should this code block be synchronized? just wondering if it should be mutually exclusive w/ the code that enables logging. server/src/main/java/org/apache/accumulo/server/watcher/MonitorLog4jWatcher.java <https://reviews.apache.org/r/17860/#comment64116> stylistic comment, feel free to ignore. Could catch NoNodeException (before catching KeeperException) and put code in if(NONODE){} in catch block - kturner On Feb. 10, 2014, 10:26 p.m., Josh Elser wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17860/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2014, 10:26 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2334 > https://issues.apache.org/jira/browse/ACCUMULO-2334 > > > Repository: accumulo > > > Description > ------- > > Removes the necessity to compute and set ACCUMULO_LOG_HOST. We can accomplish > this by having the monitor advertise a host in addition to just the port and > have the other processes place a watcher on that node in zookeeper. > > This has some nice benefits to simplifying the code, removing the annoying > (IMO) ACCUMULO_LOG_HOST variable, and, when running the monitor on a random > port, the other services can auto-discover without having to update > configuration files and restart the services. The auto-discovery also has > benefit if the monitor has to be moved to a different host (or multiple > monitors are being run with "failover") > > > Diffs > ----- > > conf/examples/1GB/native-standalone/accumulo-env.sh aa4a1d0 > conf/examples/1GB/standalone/accumulo-env.sh 1707f3d > conf/examples/2GB/native-standalone/accumulo-env.sh ef74ca7 > conf/examples/2GB/standalone/accumulo-env.sh 75014c2 > conf/examples/3GB/native-standalone/accumulo-env.sh ae0da11 > conf/examples/3GB/standalone/accumulo-env.sh 7edd938 > conf/examples/512MB/native-standalone/accumulo-env.sh 749a678 > conf/examples/512MB/standalone/accumulo-env.sh 9beb059 > core/src/main/java/org/apache/accumulo/core/Constants.java 9bb3419 > core/src/main/java/org/apache/accumulo/core/conf/Property.java 424b737 > core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java af9a1a6 > server/src/main/java/org/apache/accumulo/server/Accumulo.java f4da33b > server/src/main/java/org/apache/accumulo/server/monitor/LogService.java > 10ef9e4 > server/src/main/java/org/apache/accumulo/server/monitor/Monitor.java > dcb80fd > server/src/main/java/org/apache/accumulo/server/util/Info.java 7a07107 > server/src/main/java/org/apache/accumulo/server/util/Initialize.java > baa5400 > > server/src/main/java/org/apache/accumulo/server/watcher/MonitorLog4jWatcher.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/17860/diff/ > > > Testing > ------- > > Local testing so far. Configure the monitor to use a random log4j port, start > an instance. Then, test log forwarding works. Restart the monitor, check that > the tserver saw the update, and that log forwarding still works even though > the monitor is now listening on a different port for log4j. > > > Thanks, > > Josh Elser > >
