Code review comment for lp:~hazmat/pyjuju/test-sans-fsync

Reviewers: mp+112188_code.launchpad.net,

Message:
Please take a look.

Description:
Disable fsync on zk to speed tests.

The tests exercise zk heavily, and by defaults its configured
for per operation fsync to disk, disabling the fsync greatly
speeds up full test runs.

https://code.launchpad.net/~hazmat/juju/test-sans-fsync/+merge/112188

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6331066/

Affected files:
   A [revision details]
   M juju/lib/zk.py
   M juju/tests/common.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: juju/lib/zk.py
=== modified file 'juju/lib/zk.py'
--- juju/lib/zk.py 2011-09-22 19:32:36 +0000
+++ juju/lib/zk.py 2012-06-26 16:36:32 +0000
@@ -51,7 +51,7 @@

      def __init__(self, run_dir, port=None, host=None,
                   zk_location="system", user=None, group=None,
- use_deferred=True):
+ use_deferred=True, fsync=True):
          """
          :param run_dir: Directory to store all zookeeper instance related
data.
          :param port: The port zookeeper should run on.
@@ -73,6 +73,7 @@
          self._group = group
          self._zk_location = zk_location
          self._use_deferred = use_deferred
+ self.fsync = fsync

      def start(self):
          assert self._port is not None
@@ -197,8 +198,10 @@
              config.write(log4j_properties)

          with open(zk_vars["config_path"], "w") as config:
- config.write(
- zookeeper_conf_template % (zk_vars["data_dir"],
self._port))
+ conf = zookeeper_conf_template % (zk_vars["data_dir"],
self._port)
+ if self.fsync:
+ conf += "\nfsync=no\n"
+ config.write(conf)
              if self._host:
                  config.write("\nclientPortAddress=%s" % self._host)

Index: juju/tests/common.py
=== modified file 'juju/tests/common.py'
--- juju/tests/common.py 2011-09-16 00:36:31 +0000
+++ juju/tests/common.py 2012-06-26 16:36:32 +0000
@@ -23,7 +23,7 @@

  @contextmanager
-def zookeeper_test_context(install_path, port=28181):
+def zookeeper_test_context(install_path, port=28181, fsync=False):
      """Manage context to run/stop a ZooKeeper for testing and related vars.

      @param install_path: The path to the install for ZK. Bare word "system"
@@ -36,7 +36,8 @@
      saved_env = os.environ.get("ZOOKEEPER_ADDRESS")
      test_zookeeper = Zookeeper(
          tempfile.mkdtemp(), port,
- zk_location=install_path, use_deferred=False)
+ zk_location=install_path, use_deferred=False,
+ fsync=fsync)

      test_zookeeper.start()
      os.environ["ZOOKEEPER_ADDRESS"] = test_zookeeper.address