You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by "gosonzhang (via GitHub)" <gi...@apache.org> on 2023/06/26 03:04:21 UTC

[GitHub] [inlong] gosonzhang opened a new pull request, #8320: [INLONG-8318][DataProxy] Change notification synchronization through condition variables and locks

gosonzhang opened a new pull request, #8320:
URL: https://github.com/apache/inlong/pull/8320

   
   - Fixes #8318
   
   1. Adjust the synchronization mechanism for metadata changes of the MessageQueueZoneSink class
   2. Add log output, fix bug


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

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

[GitHub] [inlong] gosonzhang merged pull request #8320: [INLONG-8318][DataProxy] Change notification synchronization through condition variables and locks

Posted by "gosonzhang (via GitHub)" <gi...@apache.org>.
gosonzhang merged PR #8320:
URL: https://github.com/apache/inlong/pull/8320


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

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

[GitHub] [inlong] gosonzhang commented on a diff in pull request #8320: [INLONG-8318][DataProxy] Change notification synchronization through condition variables and locks

Posted by "gosonzhang (via GitHub)" <gi...@apache.org>.
gosonzhang commented on code in PR #8320:
URL: https://github.com/apache/inlong/pull/8320#discussion_r1241471333


##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/mq/MessageQueueZoneSink.java:
##########
@@ -295,8 +298,10 @@ public void update() {
         if (zoneProducer == null) {
             return;
         }
+        reentrantLock.lock();
         lastNotifyTime.set(System.currentTimeMillis());
-        syncLock.notifyAll();
+        condition.signal();
+        reentrantLock.unlock();

Review Comment:
   done



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

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

[GitHub] [inlong] luchunliang commented on a diff in pull request #8320: [INLONG-8318][DataProxy] Change notification synchronization through condition variables and locks

Posted by "luchunliang (via GitHub)" <gi...@apache.org>.
luchunliang commented on code in PR #8320:
URL: https://github.com/apache/inlong/pull/8320#discussion_r1241463657


##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/mq/MessageQueueZoneSink.java:
##########
@@ -295,8 +298,10 @@ public void update() {
         if (zoneProducer == null) {
             return;
         }
+        reentrantLock.lock();
         lastNotifyTime.set(System.currentTimeMillis());
-        syncLock.notifyAll();
+        condition.signal();
+        reentrantLock.unlock();

Review Comment:
   It will have problem if condition.signal() have an exception.
   reentrantLock.unlock() need a finally block.



##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/mq/MessageQueueZoneSink.java:
##########
@@ -311,14 +316,16 @@ private class ConfigChangeProcessor implements Runnable {
         @Override
         public void run() {
             long lastCheckTime;
+            logger.info("{} config-change processor start!", getName());
             while (!isShutdown) {
+                reentrantLock.lock();

Review Comment:
   When updating metadata, it is not necessary using lock.



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

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

[GitHub] [inlong] gosonzhang commented on a diff in pull request #8320: [INLONG-8318][DataProxy] Change notification synchronization through condition variables and locks

Posted by "gosonzhang (via GitHub)" <gi...@apache.org>.
gosonzhang commented on code in PR #8320:
URL: https://github.com/apache/inlong/pull/8320#discussion_r1241474228


##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/mq/MessageQueueZoneSink.java:
##########
@@ -311,14 +316,16 @@ private class ConfigChangeProcessor implements Runnable {
         @Override
         public void run() {
             long lastCheckTime;
+            logger.info("{} config-change processor start!", getName());
             while (!isShutdown) {
+                reentrantLock.lock();

Review Comment:
   This lock with the condition is used to coordinate the message notification between the configuration change notification thread and the configuration update thread in conjunction 



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

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