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/01/22 06:17:33 UTC

[GitHub] [pulsar] merlimat opened a new pull request #9273: PIP-45: Added session events to metadata store

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


   ### Motivation
   
   Added mechanism to send notification on the metadata store session status. This will be used to trigger lock revalidation after a session is lost and also to do best effort approach in avoiding attempting non-essential metadata operations while the client is known not to be connected.


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

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



[GitHub] [pulsar] merlimat commented on a change in pull request #9273: PIP-45: Added session events to metadata store

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



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java
##########
@@ -205,6 +207,21 @@ public void accept(Notification n) {
                 });
     }
 
+    @Override
+    public void registerSessionListener(Consumer<SessionEvent> listener) {
+        sessionListeners.add(listener);
+    }
+
+    protected void receivedSessionEvent(SessionEvent event) {
+        sessionListeners.forEach(l -> {
+            try {
+                l.accept(event);
+            } catch (Throwable t) {
+                log.warn("Error in processing session event", t);

Review comment:
       The `Consumer` interface doesn't allow to throw `InterruptedException` and we cannot catch that directly since it's a checked exception. If the listener code does any blocking operation, it would be its responsibility to handle the `InterruptedException`.




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

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



[GitHub] [pulsar] merlimat merged pull request #9273: PIP-45: Added session events to metadata store

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


   


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

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



[GitHub] [pulsar] merlimat commented on a change in pull request #9273: PIP-45: Added session events to metadata store

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



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java
##########
@@ -205,6 +207,21 @@ public void accept(Notification n) {
                 });
     }
 
+    @Override
+    public void registerSessionListener(Consumer<SessionEvent> listener) {
+        sessionListeners.add(listener);
+    }
+
+    protected void receivedSessionEvent(SessionEvent event) {
+        sessionListeners.forEach(l -> {
+            try {
+                l.accept(event);
+            } catch (Throwable t) {
+                log.warn("Error in processing session event", t);

Review comment:
       The `Consumer` interface doesn't allow to throw `InterruptedException` and we cannot catch that directly since it's a checked exception. If the listener code does any blocking operation, it would be its responsibility to handle the `InterruptedException`.




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

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



[GitHub] [pulsar] rdhabalia commented on pull request #9273: PIP-45: Added session events to metadata store

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #9273:
URL: https://github.com/apache/pulsar/pull/9273#issuecomment-771268713


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] jiazhai commented on pull request #9273: PIP-45: Added session events to metadata store

Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #9273:
URL: https://github.com/apache/pulsar/pull/9273#issuecomment-770513274


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] rdhabalia commented on pull request #9273: PIP-45: Added session events to metadata store

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #9273:
URL: https://github.com/apache/pulsar/pull/9273#issuecomment-771268713


   /pulsarbot run-failure-checks


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

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #9273: PIP-45: Added session events to metadata store

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



##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java
##########
@@ -205,6 +207,21 @@ public void accept(Notification n) {
                 });
     }
 
+    @Override
+    public void registerSessionListener(Consumer<SessionEvent> listener) {
+        sessionListeners.add(listener);
+    }
+
+    protected void receivedSessionEvent(SessionEvent event) {
+        sessionListeners.forEach(l -> {
+            try {
+                l.accept(event);
+            } catch (Throwable t) {
+                log.warn("Error in processing session event", t);

Review comment:
       InterruptedException?




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

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