[GitHub] [pulsar] codelipenghui commented on a diff in pull request #15757: Issue 15750: PIP-105: Store Subscription properties

[GitBox]

codelipenghui commented on code in PR #15757:
URL: https://github.com/apache/pulsar/pull/15757#discussion_r881347141


##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedCursor.java:
##########
@@ -79,6 +81,20 @@ enum IndividualDeletedEntries {
      */
     Map<String, Long> getProperties();
 
+    /**
+     * Return any properties that were associated with the cursor.
+     */
+    Map<String, String> getCursorProperties();
+
+     /**
+      * Updates the properties
+      * @param cursorProperties
+      * @return a handle to the result of the operation
+      */
+     default CompletableFuture<Void> updateCursorProperties(Map<String, 
String> cursorProperties) {

Review Comment:
   ```suggestion
        default CompletableFuture<Void> setCursorProperties(Map<String, String> 
cursorProperties) {
   ```



##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedger.java:
##########
@@ -243,7 +243,8 @@ ManagedCursor openCursor(String name, InitialPosition 
initialPosition) throws In
      * @return the ManagedCursor
      * @throws ManagedLedgerException
      */
-    ManagedCursor openCursor(String name, InitialPosition initialPosition, 
Map<String, Long> properties)
+    ManagedCursor openCursor(String name, InitialPosition initialPosition, 
Map<String, Long> properties,
+                             Map<String, String> ledgerProperties)

Review Comment:
   ```suggestion
       ManagedCursor openCursor(String name, InitialPosition initialPosition, 
Map<String, Long> properties,
                                Map<String, String> cursorProperties)
   ```



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:
##########
@@ -898,21 +900,34 @@ public void openCursorComplete(ManagedCursor cursor, 
Object ctx) {
                         return;
                     }
                 }
+                CompletableFuture<Subscription> updateProperties;
+                PersistentSubscription _subscription = subscription;
+
                 if (MapUtils.isEmpty(subscription.getSubscriptionProperties())
                         && MapUtils.isNotEmpty(subscriptionProperties)) {
-                    
subscription.getSubscriptionProperties().putAll(subscriptionProperties);
+                    updateProperties = subscription
+                            
.updateSubscriptionProperties(subscriptionProperties)
+                            .thenApply(___ -> _subscription);

Review Comment:
   I have left a comment here 
https://github.com/apache/pulsar/pull/15757#issuecomment-1136921453



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org