uploader: fix rate limiting for logdir polling

Summary:
Recent refactorings to the RPC rate limiting had the unintended side
effect of removing rate limiting on logdir polling. This meant that
after all data had been read from the logdir, the uploader would
continue to poll it aggressively (thousands of times per second on my
machine), which is bad for disks and expensive on network file systems.

This commit adds a separate rate limiter for the logdir polling. We
don’t reuse the RPC rate limiter because that would force us to tick
twice on the same timer every cycle.

Fixes #3001.

Test Plan:
Run `bazel run //tensorboard -- dev upload --logdir /nope --verbosity 0`
(with any logdir path, but it’s easier when it’s empty), and note that
the “Starting an upload cycle” logs now progress at 0.2 per second
rather than 6000 per second. TODO: Unit tests.

wchargin-branch: uploader-rate-limit-polling
wchargin-source: 2036f1c749e0c2f4e6db2362257e1f3f3a4342f2

Author: wchargin

tensorboard/uploader/uploader.py

@@ -38,6 +38,11 @@
 from tensorboard.util import tb_logging
 from tensorboard.util import tensor_util
 
+# Minimum length of a logdir polling cycle in seconds. Shorter cycles will
+# sleep to avoid spinning over the logdir, which isn't great for disks and can
+# be expensive for network file sytems.
+_MIN_LOGDIR_POLL_INTERVAL_SECS = 5
+
 # Minimum interval between initiating write RPCs.  When writes would otherwise
 # happen more frequently, the process will sleep to use up the rest of the time.
 _MIN_WRITE_RPC_INTERVAL_SECS = 5
@@ -70,6 +75,7 @@ def __init__(
         writer_client,
         logdir,
         allowed_plugins,
+        logdir_poll_rate_limiter=None,
         rpc_rate_limiter=None,
         name=None,
         description=None,
@@ -82,6 +88,9 @@ def __init__(
           allowed_plugins: collection of string plugin names; events will only
             be uploaded if their time series's metadata specifies one of these
             plugin names
+          logdir_poll_rate_limiter: a `RateLimiter` to use to limit logdir
+            polling frequency, to avoid thrashing disks, especially on networked
+            file systems.
           rpc_rate_limiter: a `RateLimiter` to use to limit write RPC frequency.
             Note this limit applies at the level of single RPCs in the Scalar
             and Tensor case, but at the level of an entire blob upload in the
@@ -98,6 +107,12 @@ def __init__(
         self._name = name
         self._description = description
         self._request_sender = None
+        if logdir_poll_rate_limiter is None:
+            self._logdir_poll_rate_limiter = util.RateLimiter(
+                _MIN_LOGDIR_POLL_INTERVAL_SECS
+            )
+        else:
+            self._logdir_poll_rate_limiter = logdir_poll_rate_limiter
         if rpc_rate_limiter is None:
             self._rpc_rate_limiter = util.RateLimiter(
                 _MIN_WRITE_RPC_INTERVAL_SECS
@@ -147,6 +162,7 @@ def start_uploading(self):
                 "Must call create_experiment() before start_uploading()"
             )
         while True:
+            self._logdir_poll_rate_limiter.tick()
             self._upload_once()
 
     def _upload_once(self):

tensorboard/uploader/uploader_test.py

@@ -81,6 +81,7 @@ def _create_uploader(
     writer_client=_USE_DEFAULT,
     logdir=None,
     allowed_plugins=_USE_DEFAULT,
+    logdir_poll_rate_limiter=_USE_DEFAULT,
     rpc_rate_limiter=_USE_DEFAULT,
     name=None,
     description=None,
@@ -89,12 +90,15 @@ def _create_uploader(
         writer_client = _create_mock_client()
     if allowed_plugins is _USE_DEFAULT:
         allowed_plugins = _SCALARS_ONLY
+    if logdir_poll_rate_limiter is _USE_DEFAULT:
+        logdir_poll_rate_limiter = util.RateLimiter(0)
     if rpc_rate_limiter is _USE_DEFAULT:
         rpc_rate_limiter = util.RateLimiter(0)
     return uploader_lib.TensorBoardUploader(
         writer_client,
         logdir,
         allowed_plugins=allowed_plugins,
+        logdir_poll_rate_limiter=logdir_poll_rate_limiter,
         rpc_rate_limiter=rpc_rate_limiter,
         name=name,
         description=description,