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 2021/09/29 18:55:58 UTC

[GitHub] [pulsar] merlimat opened a new pull request #12243: PIP-45: Allow more info in MetadataSerde

merlimat opened a new pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243


   ### Motivation
   
   In some cases, at the moment of doing serialization/deserialization, we need to use more information than just the `byte[]` payload. For example this happens in the context of BK abstract interfaces for metadata access. 
   
   Adding here the support for passing the path and stats of the metadata value to the serializer, which it can then either use or ignore.


-- 
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



[GitHub] [pulsar] merlimat commented on a change in pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243#discussion_r718854182



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/CoordinationServiceImpl.java
##########
@@ -80,6 +81,12 @@ public void close() throws Exception {
                 k -> new LockManagerImpl<T>(store, clazz, executor));
     }
 
+    @Override
+    public <T> LockManager<T> getLockManager(MetadataSerde<T> serde) {
+        return (LockManager<T>) lockManagers.computeIfAbsent(serde,

Review comment:
       Oh, I didn't think of that. The thing is that the caching of the instances of `LockManager` is not very useful in practice (since we mostly call it once..). It was just there to have a collection of instances to close when we close the LockManager. In that light, the current approach could be "fine". 




-- 
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



[GitHub] [pulsar] eolivelli merged pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243


   


-- 
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



[GitHub] [pulsar] eolivelli commented on a change in pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243#discussion_r718870503



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/CoordinationServiceImpl.java
##########
@@ -80,6 +81,12 @@ public void close() throws Exception {
                 k -> new LockManagerImpl<T>(store, clazz, executor));
     }
 
+    @Override
+    public <T> LockManager<T> getLockManager(MetadataSerde<T> serde) {
+        return (LockManager<T>) lockManagers.computeIfAbsent(serde,

Review comment:
       Makes sense to me. Thanks




-- 
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



[GitHub] [pulsar] eolivelli merged pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243


   


-- 
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



[GitHub] [pulsar] eolivelli commented on a change in pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243#discussion_r718847640



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/CoordinationServiceImpl.java
##########
@@ -80,6 +81,12 @@ public void close() throws Exception {
                 k -> new LockManagerImpl<T>(store, clazz, executor));
     }
 
+    @Override
+    public <T> LockManager<T> getLockManager(MetadataSerde<T> serde) {
+        return (LockManager<T>) lockManagers.computeIfAbsent(serde,

Review comment:
       So we require to implement equals and hashCode on MetadataSerde now?

##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/CoordinationServiceImpl.java
##########
@@ -80,6 +81,12 @@ public void close() throws Exception {
                 k -> new LockManagerImpl<T>(store, clazz, executor));
     }
 
+    @Override
+    public <T> LockManager<T> getLockManager(MetadataSerde<T> serde) {
+        return (LockManager<T>) lockManagers.computeIfAbsent(serde,

Review comment:
       Makes sense to me. Thanks




-- 
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



[GitHub] [pulsar] eolivelli commented on a change in pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243#discussion_r718847640



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/CoordinationServiceImpl.java
##########
@@ -80,6 +81,12 @@ public void close() throws Exception {
                 k -> new LockManagerImpl<T>(store, clazz, executor));
     }
 
+    @Override
+    public <T> LockManager<T> getLockManager(MetadataSerde<T> serde) {
+        return (LockManager<T>) lockManagers.computeIfAbsent(serde,

Review comment:
       So we require to implement equals and hashCode on MetadataSerde now?




-- 
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



[GitHub] [pulsar] merlimat commented on a change in pull request #12243: PIP-45: Allow more info in MetadataSerde

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12243:
URL: https://github.com/apache/pulsar/pull/12243#discussion_r718854182



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/coordination/impl/CoordinationServiceImpl.java
##########
@@ -80,6 +81,12 @@ public void close() throws Exception {
                 k -> new LockManagerImpl<T>(store, clazz, executor));
     }
 
+    @Override
+    public <T> LockManager<T> getLockManager(MetadataSerde<T> serde) {
+        return (LockManager<T>) lockManagers.computeIfAbsent(serde,

Review comment:
       Oh, I didn't think of that. The thing is that the caching of the instances of `LockManager` is not very useful in practice (since we mostly call it once..). It was just there to have a collection of instances to close when we close the LockManager. In that light, the current approach could be "fine". 




-- 
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