|
|
|
Increase zk session and ping times.
During scale testing work, one of the take aways was the default zookeeper
session time was too low, which caused issues for any transient communication
problems between the agents the zk. The managed client work landed subsequently
does recovery for this and for session expiration, but its better to avoid
the issue entirely by increasing the session time and increasing the heartbeat
time periodicity.
https://code.launchpad.net/~hazmat/juju/increase-session-timeout/+merge/146169
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 8
Patch Set 2 : Increase zk session and ping times. #Patch Set 3 : Increase zk session and ping times. #
Total messages: 6
|
hazmat
Please take a look.
|
12 years, 11 months ago (2013年02月01日 17:01:26 UTC) #1 | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Please take a look.
LGTM, some minors below but they are optional. https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py File juju/lib/port.py (right): https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py#newcode5 juju/lib/port.py:5: """Get an open port on the machine. There is a race between close and return, but the kernel shouldn't re-allocate the same port number again for some reasonable window so this should be ok. https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py File juju/lib/zk.py (right): https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py#newcode57 juju/lib/zk.py:57: use_deferred=True, min_session_timeout=30000, Should you use the constant above for min and define one for max and use it as well? https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit.py File juju/providers/common/cloudinit.py (right): https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit... juju/providers/common/cloudinit.py:46: 'echo "maxSessionTimeout=%d" >> /etc/zookeeper/conf/zoo.cfg' % ( It might make sense that these are defined in lib.zk rather than calculated values here. https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.py File juju/providers/common/connect.py (right): https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.p... juju/providers/common/connect.py:67: client = yield SSHClient(session_timeout=20000).connect( Should this be the default in SSHClient.__init__?
thanks for the review. https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py File juju/lib/port.py (right): https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py#newcode5 juju/lib/port.py:5: """Get an open port on the machine. On 2013年02月01日 17:12:20, bcsaller wrote: > There is a race between close and return, but the kernel shouldn't re-allocate > the same port number again for some reasonable window so this should be ok. this is a refactor from elsewhere, the so reuse i believe helps with a concurrent handout, in either case better than a hardcode. https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py File juju/lib/zk.py (right): https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py#newcode57 juju/lib/zk.py:57: use_deferred=True, min_session_timeout=30000, On 2013年02月01日 17:12:20, bcsaller wrote: > Should you use the constant above for min and define one for max and use it as > well? makes sense, done. https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit.py File juju/providers/common/cloudinit.py (right): https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit... juju/providers/common/cloudinit.py:46: 'echo "maxSessionTimeout=%d" >> /etc/zookeeper/conf/zoo.cfg' % ( On 2013年02月01日 17:12:20, bcsaller wrote: > It might make sense that these are defined in lib.zk rather than calculated > values here. done. https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.py File juju/providers/common/connect.py (right): https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.p... juju/providers/common/connect.py:67: client = yield SSHClient(session_timeout=20000).connect( On 2013年02月01日 17:12:20, bcsaller wrote: > Should this be the default in SSHClient.__init__? sshclient doesn't define that, changed to use the lib.zk CLIENT_SESSION_TIME
Please take a look.
Thanks for the changes, LGTM
*** Submitted: Increase zk session and ping times. During scale testing work, one of the take aways was the default zookeeper session time was too low, which caused issues for any transient communication problems between the agents the zk. The managed client work landed subsequently does recovery for this and for session expiration, but its better to avoid the issue entirely by increasing the session time and increasing the heartbeat time periodicity. R=bcsaller CC= https://codereview.appspot.com/7231079