uuid: Improve architecture rationale docs for GlobalIdGenerator

Krinkle · AaronSchulz · commit 44a343463162 · 2021-02-18T22:17:32.000Z
- Fix typos and grammer.

- Fix indentation.

- Remove "The drift is immediate" sentence. It doesn't read well,
  and I don't remember what I meant with it.

Change-Id: I9c3dc69db437644f677cbb3797f7db40c5ec5eeb
diff --git a/includes/libs/uuid/GlobalIdGenerator.php b/includes/libs/uuid/GlobalIdGenerator.php
@@ -464,7 +464,7 @@ protected function getTimeAndDelay( $lockFile, $clockSeqSize, $counterSize, $off
 		// cannot increment further, because the formatted ID would not have enough
 		// bits to fit the counter.
 		//
-		// To orchestrate this between independant PHP processes on the same hosts,
+		// To orchestrate this between independent PHP processes on the same host,
 		// we must have a common sense of time so that we only have to maintain
 		// a single counter in a single lock file.
 		//
@@ -473,31 +473,31 @@ protected function getTimeAndDelay( $lockFile, $clockSeqSize, $counterSize, $off
 		// * Some other clock can be observed via microtime(), which also offers
 		//   millisecond precision.
 		// * microtime() drifts in-process further and further away from the system
-		//   clock the longer the longer the process runs for.
+		//   clock the longer a process runs for.
 		//   For example, on 2018-10-03 an HHVM 3.18 JobQueue process at WMF,
-		//   that ran for 9 min 55 sec, drifted 7 seconds.
-		//   The drift is immediate for processes running while the system clock changes.
+		//   that ran for 9 min 55 sec, microtime drifted by 7 seconds.
 		//   time() does not have this problem. See https://bugs.php.net/bug.php?id=42659.
 		//
 		// We have two choices:
 		//
 		// 1. Use microtime() with the following caveats:
-		//    - The last stored time may be in the future, or our current time
-		//    may be in the past, in which case we'll frequently enter the slow
-		//    timeWaitUntil() method to try and "sync" the current process with
-		//    the previous process. We mustn't block for long though, max 10ms?
-		//    - For any drift above 10ms, we pretend that the clock went backwards,
-		//    and treat it the same way as after an NTP sync, by incrementing clock
-		//    sequence instead. Given this rolls over automatically and silently
-		//    and is meant to be rare, this is essentially sacrifices a reasonable
-		//    guarantee of uniqueness.
-		//    - For long running processes (e.g. longer than a few seconds) the drift
-		//    can easily be more than 2 seconds. Because we only have a single lock
-		//    file and don't want to keep too many counters and deal with clearing
-		//    those, we fatal the user and refuse to make an ID. (T94522)
-		// 2. Use time() and expand the counter by 1000x and use the first digits
-		//    as if they are the millisecond fraction of the timestamp.
-		//    Known caveats or perf impact: None.
+		//    - The last stored time may be in the future, or our current time may be in the
+		//      past, in which case we'll frequently enter the slow timeWaitUntil() method to
+		//      try and "sync" the current process with the previous process.
+		//      We mustn't block for long though, max 10ms?
+		//    - For any drift above 10ms, we pretend that the clock went backwards, and treat
+		//      it the same way as after an NTP sync, by incrementing clock sequence instead.
+		//      Given the sequence rolls over automatically, and silently, and is meant to be
+		//      rare, this is essentially sacrifices a reasonable guarantee of uniqueness.
+		//    - For long running processes (e.g. longer than a few seconds) the drift can
+		//      easily be more than 2 seconds. Because we only have a single lock file
+		//      and don't want to keep too many counters and deal with clearing those,
+		//      we fatal the user and refuse to make an ID.  (T94522)
+		//    - This offers terrible service availability.
+		// 2. Use time() instead, and expand the counter size by 1000x and use its
+		//    digits as if they were the millisecond fraction of our timestamp.
+		//    Known caveats or perf impact: None. We still need to read-write our
+		//    lock file on each generation, so might as well make the most of it.
 		//
 		// We choose the latter.
 		$msecCounterSize = $counterSize * 1000;
