Synchronizing populatePropertyBag as additional fallback to avoid tra…

…nsient invalid json in Changefeed state (#21020) It is not fully clear yet how we occasionally end-up with invalid json for the ChangeFeedStartFrom settings (json would have "Type": "Now", "Type": "Now") - the populate property bag method uses Map<>.put (LinkedHashMap) to add the property - which should never result in duplicates. Synchronizing this part is just an attempt to reduce risk - not because there is a known multi-threaded usage that could result in the duplicates.
Azure · Apr 29, 2021 · 3f6d3c8 · 3f6d3c8
1 parent 98f1e6b
commit 3f6d3c8
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 32 deletions.
diff --git a/...ure/cosmos/implementation/changefeed/implementation/ChangeFeedStartFromBeginningImpl.java b/...ure/cosmos/implementation/changefeed/implementation/ChangeFeedStartFromBeginningImpl.java
@@ -16,10 +16,12 @@ public ChangeFeedStartFromBeginningImpl() {
     public void populatePropertyBag() {
         super.populatePropertyBag();
 
-        setProperty(
-            this,
-            Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
-            ChangeFeedStartFromTypes.BEGINNING);
+        synchronized(this) {
+            setProperty(
+                this,
+                Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
+                ChangeFeedStartFromTypes.BEGINNING);
+        }
     }
 
     @Override

diff --git a/...mos/implementation/changefeed/implementation/ChangeFeedStartFromETagAndFeedRangeImpl.java b/...mos/implementation/changefeed/implementation/ChangeFeedStartFromETagAndFeedRangeImpl.java
@@ -82,19 +82,21 @@ public void populateRequest(RxDocumentServiceRequest request) {
     public void populatePropertyBag() {
         super.populatePropertyBag();
 
-        setProperty(
-            this,
-            Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
-            ChangeFeedStartFromTypes.LEASE);
-
-        setProperty(
-            this,
-            Constants.Properties.CHANGE_FEED_START_FROM_ETAG,
-            this.eTag);
-
-        this.feedRange.setProperties(
-            this,
-            true);
+        synchronized(this) {
+            setProperty(
+                this,
+                Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
+                ChangeFeedStartFromTypes.LEASE);
+
+            setProperty(
+                this,
+                Constants.Properties.CHANGE_FEED_START_FROM_ETAG,
+                this.eTag);
+
+            this.feedRange.setProperties(
+                this,
+                true);
+        }
     }
 
     @Override

diff --git a/...com/azure/cosmos/implementation/changefeed/implementation/ChangeFeedStartFromNowImpl.java b/...com/azure/cosmos/implementation/changefeed/implementation/ChangeFeedStartFromNowImpl.java
@@ -16,12 +16,15 @@ public ChangeFeedStartFromNowImpl() {
 
     @Override
     public void populatePropertyBag() {
-        super.populatePropertyBag();
-
-        setProperty(
-            this,
-            Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
-            ChangeFeedStartFromTypes.NOW);
+
+            super.populatePropertyBag();
+
+        synchronized(this) {
+            setProperty(
+                this,
+                Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
+                ChangeFeedStartFromTypes.NOW);
+        }
     }
 
     @Override

diff --git a/...e/cosmos/implementation/changefeed/implementation/ChangeFeedStartFromPointInTimeImpl.java b/...e/cosmos/implementation/changefeed/implementation/ChangeFeedStartFromPointInTimeImpl.java
@@ -49,15 +49,17 @@ public void populatePropertyBag() {
 
         super.populatePropertyBag();
 
-        setProperty(
-            this,
-            com.azure.cosmos.implementation.Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
-            ChangeFeedStartFromTypes.POINT_IN_TIME);
-
-        setProperty(
-            this,
-            Constants.Properties.CHANGE_FEED_START_FROM_POINT_IN_TIME_MS,
-            this.pointInTime.toEpochMilli());
+        synchronized(this) {
+            setProperty(
+                this,
+                com.azure.cosmos.implementation.Constants.Properties.CHANGE_FEED_START_FROM_TYPE,
+                ChangeFeedStartFromTypes.POINT_IN_TIME);
+
+            setProperty(
+                this,
+                Constants.Properties.CHANGE_FEED_START_FROM_POINT_IN_TIME_MS,
+                this.pointInTime.toEpochMilli());
+        }
     }
 
     @Override