From f81853ed530771ac600bca87d4d2593bf05ff39c Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 15 May 2018 15:33:38 -0700
Subject: [PATCH] objectcache: add BagOStuff comment additions about access
 scope

Change-Id: Id23859a58ea3bde0338ba4d22ce12ffcbbf4480a
---
 includes/libs/objectcache/BagOStuff.php     | 27 +++++++++++++++------
 includes/libs/rdbms/ChronologyProtector.php | 11 +++++----
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php
index 8420f113ff..8a88581026 100644
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -33,14 +33,25 @@ use Wikimedia\ScopedCallback;
 use Wikimedia\WaitConditionLoop;
 
 /**
- * interface is intended to be more or less compatible with
- * the PHP memcached client.
+ * Class representing a cache/ephemeral data store
  *
- * backends for local hash array and SQL table included:
- * @code
- *   $bag = new HashBagOStuff();
- *   $bag = new SqlBagOStuff(); # connect to db first
- * @endcode
+ * This interface is intended to be more or less compatible with the PHP memcached client.
+ *
+ * Instances of this class should be created with an intended access scope, such as:
+ *   - a) A single PHP thread on a server (e.g. stored in a PHP variable)
+ *   - b) A single application server (e.g. stored in APC or sqlite)
+ *   - c) All application servers in datacenter (e.g. stored in memcached or mysql)
+ *   - d) All application servers in all datacenters (e.g. stored via mcrouter or dynomite)
+ *
+ * Callers should use the proper factory methods that yield BagOStuff instances. Site admins
+ * should make sure the configuration for those factory methods matches their access scope.
+ * BagOStuff subclasses have widely varying levels of support for replication features.
+ *
+ * For any given instance, methods like lock(), unlock(), merge(), and set() with WRITE_SYNC
+ * should semantically operate over its entire access scope; any nodes/threads in that scope
+ * should serialize appropriately when using them. Likewise, a call to get() with READ_LATEST
+ * from one node in its access scope should reflect the prior changes of any other node its access
+ * scope. Any get() should reflect the changes of any prior set() with WRITE_SYNC.
  *
  * @ingroup Cache
  */
@@ -165,7 +176,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
 	/**
 	 * Get an item with the given key
 	 *
-	 * If the key includes a determistic input hash (e.g. the key can only have
+	 * If the key includes a deterministic input hash (e.g. the key can only have
 	 * the correct value) or complete staleness checks are handled by the caller
 	 * (e.g. nothing relies on the TTL), then the READ_VERIFIED flag should be set.
 	 * This lets tiered backends know they can safely upgrade a cached value to
diff --git a/includes/libs/rdbms/ChronologyProtector.php b/includes/libs/rdbms/ChronologyProtector.php
index 90e697ec16..186014b7a2 100644
--- a/includes/libs/rdbms/ChronologyProtector.php
+++ b/includes/libs/rdbms/ChronologyProtector.php
@@ -190,13 +190,14 @@ class ChronologyProtector implements LoggerAwareInterface {
 			implode( ', ', array_keys( $this->shutdownPositions ) ) . "\n"
 		);
 
-		// CP-protected writes should overwhemingly go to the master datacenter, so get DC-local
-		// lock to merge the values. Use a DC-local get() and a synchronous all-DC set(). This
-		// makes it possible for the BagOStuff class to write in parallel to all DCs with one RTT.
+		// CP-protected writes should overwhelmingly go to the master datacenter, so use a
+		// DC-local lock to merge the values. Use a DC-local get() and a synchronous all-DC
+		// set(). This makes it possible for the BagOStuff class to write in parallel to all
+		// DCs with one RTT. The use of WRITE_SYNC avoids needing READ_LATEST for the get().
 		if ( $store->lock( $this->key, 3 ) ) {
 			if ( $workCallback ) {
-				// Let the store run the work before blocking on a replication sync barrier. By the
-				// time it's done with the work, the barrier should be fast if replication caught up.
+				// Let the store run the work before blocking on a replication sync barrier.
+				// If replication caught up while the work finished, the barrier will be fast.
 				$store->addBusyCallback( $workCallback );
 			}
 			$ok = $store->set(
-- 
2.20.1