> On Feb. 8, 2014, 12:48 a.m., kturner wrote: > > server/src/main/java/org/apache/accumulo/server/watcher/MonitorLog4jWatcher.java, > > line 95 > > <https://reviews.apache.org/r/17860/diff/1/?file=480379#file480379line95> > > > > If getData() throws NoNodeException (or returns null? not sure if that > > will happen) should probably gracefully handle this and stop trying to send > > log data wherever it used to be sending it. > > Josh Elser wrote: > This was another direct copy/paste from what was previously there. Is > there a race condition from a node going away that you have a Watcher set on? > The initialization does check that the path exists before initially > configuring log4j. > > When the monitor goes away (and "quiteMode" is turned on the logger) we > just ignore the errors about not being able to talk to the remote and then > the messages will eventually get dropped due to the configuration. This is > the same thing as what presently happens. I have no idea if we would have any > noticeable positive impact to disabling that appender (or similar) when we > see that the logger goes away rather than just letting the framework roll > away those messages.
I am not thinking of a race condition, just thinking of the condition where the node is there and then later goes away. If it were an ephemeral node, it would go away when the monitor processes died. Zookeeper will call the watcher when the node goes away. Then because exist is called, the watcher will be called if it ever comes back. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17860/#review33988 ----------------------------------------------------------- On Feb. 7, 2014, 10:30 p.m., Josh Elser wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17860/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2014, 10:30 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/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/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 > >
