Attempt at describing the leaky bucket thread

ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Util/LeakyBucket.hs

@@ -215,7 +215,7 @@ runAgainstBucket ::
   (Handlers m -> m a) ->
   m (State m, a)
 runAgainstBucket config action = do
-  runThreadVar <- atomically newEmptyTMVar
+  runThreadVar <- atomically newEmptyTMVar -- see note [Leaky bucket design].
   tid <- myThreadId
   bucket <- init config
   withAsync (leak runThreadVar tid bucket) $ \_ -> do
@@ -289,8 +289,44 @@ init config@Config {capacity} = do
         config = config
       }
 
+-- Note [Leaky bucket design]
+-- ~~~~~~~~~~~~~~~~~~~~~~~~~~
+--
+-- The leaky bucket works by running the given action against a thread that
+-- makes the bucket leak. Since that would be extremely inefficient to actually
+-- remove tokens one by one from the token, the 'leak' thread instead looks at
+-- the current state of the bucket, computes how much time it would take for the
+-- bucket to empty, and then wait that amount of time. Once the wait is over, it
+-- recurses, looks at the new state of the bucket, etc. If tokens were given to
+-- the bucket via the action, the bucket is not empty and the loop continues.
+--
+-- This description assumes that two things hold:
+--
+--  - the bucket must be leaking (ie. rate is strictly positive),
+--  - the action can only increase the waiting time (eg. by giving tokens).
+--
+-- Neither of those properties hold in the general case. Indeed, it is possible
+-- for the bucket to have a zero rate or even a negative one (for a more
+-- traditional rate limiting bucket, for instance). Conversely, it is possible
+-- for the action to lower the waiting time by changing the bucket configuration
+-- to one where the rate is higher.
+--
+-- We fix both those issues with one mechanism, the “runThreadVar”. It is an
+-- MVar containing an integer that tells the thread whether it should be
+-- running. An empty MVar means that the thread should not be running, for
+-- instance if the rate is null. A full MVar (no matter what the integer is)
+-- means that the thread should be running. When recursing, the thread blocks
+-- until the MVar is full, and only then proceeds as described above.
+-- Additionally, while waiting for the bucket to empty, the thread monitors
+-- changes to the MVar, indicating either that the thread should stop running or
+-- that the configuration changed as that it might have to wait less long. The
+-- change in configuration is detected by changes in the integer.
+--
+-- Note that we call “start”/“stop” running the action of filling/emtpying the
+-- MVar. This is not to mistaken for the thread actually being spawned/killed.
+
 -- | Monadic action that calls 'threadDelay' until the bucket is empty, then
--- runs the 'onEmpty' action and terminates.
+-- runs the 'onEmpty' action and terminates. See note [Leaky bucket design].
 leak ::
   ( MonadDelay m,
     MonadCatch m,