accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 17860: Review of changes that remove the necessity for ACCUMULO_LOG_HOST
Date Tue, 11 Feb 2014 17:15:43 GMT


> On Feb. 10, 2014, 11:24 p.m., kturner wrote:
> > server/src/main/java/org/apache/accumulo/server/monitor/Monitor.java, line 554
> > <https://reviews.apache.org/r/17860/diff/2-3/?file=481200#file481200line554>
> >
> >     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.
> 
> Josh Elser wrote:
>     Yes. I tested it against a 1.5.1-SNAPSHOT install I had locally that did not yet
have these changes to ZooKeeper and it worked just fine.
>     
>     I can move it over to the Master code. I'll look at doing that to make sure it makes
sense.
> 
> kturner wrote:
>     there is a function called upgradeZookeeper() in Master
> 
> Josh Elser wrote:
>     I don't think it makes sense to move the code into the Master. As it stands now,
we can trigger the zookeeper check when the monitor starts up. If we move it to the monitor,
we'd have to wait for the Master to come up and finish the upgrade, and then start. I think
it's simpler to just leave it in the Monitor.

I agree w/ what you said.    Maybe a better solution is to put all of the upgrade code in
one package or class.  Master can call code here and monitor can call method here to upgrade
itself.  This would put all of the upgrade related code in one place making it easier for
future upgrade related changes.    Or maybe put a comment in the master upgrade code that
mentions there is also upgrade code in the monitor, so at least there is a pointer to it.
 M


- kturner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17860/#review34124
-----------------------------------------------------------


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message