You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/05/25 08:14:00 UTC

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

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-unsubscribe@pulsar.apache.org

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