Merge lp:~hazmat/pyjuju/test-sans-fsync into lp:pyjuju

[Kapil Thangavelu]

disable fsync on zk to speed tests.

[r=bcsaller,clint-fewbar]

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://codereview.appspot.com/6331066/

[Kapil Thangavelu (hazmat)]

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

[Benjamin Saller (bcsaller)]

LGTM +1 and a good idea too

review: Approve

[Clint Byrum (clint-fewbar)]

Tested, works great. Thanks Kapil!

review: Approve

[Kapil Thangavelu (hazmat)]

*** Submitted:

disable fsync on zk to speed tests.

[r=bcsaller,clint-fewbar]

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.

R=
CC=
https://codereview.appspot.com/6331066

https://codereview.appspot.com/6331066/

=== modified file 'juju/lib/zk.py'
--- juju/lib/zk.py	2011-09-22 19:32:36 +0000
+++ juju/lib/zk.py	2012-06-26 18:45:49 +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)
 

=== modified file 'juju/tests/common.py'
--- juju/tests/common.py	2011-09-16 00:36:31 +0000
+++ juju/tests/common.py	2012-06-26 18:45:49 +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