You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/08/23 10:34:09 UTC

[GitHub] [bookkeeper] yapxue opened a new pull request, #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

yapxue opened a new pull request, #3454:
URL: https://github.com/apache/bookkeeper/pull/3454

   Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread
   
   ### Motivation
   Bookkeeper 4.15 has performance issue. There are too many journal sync.
   Then I found in ForceWriteThread, forceWriteMarkerSent is reset to false for every loop, and this can cause shouldForceWrite=true and trigger journal sync. This new logic is bringed by https://github.com/apache/bookkeeper/pull/2962
   
   ### Changes
   move forceWriteMarkerSent=false out of while loop.
   
   


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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454#issuecomment-1223906654

   @zymap @hangc0276 @eolivelli  have a look,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@bookkeeper.apache.org

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


[GitHub] [bookkeeper] zymap commented on pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
zymap commented on PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454#issuecomment-1223942429

   > Is it able to add a test to avoid the regression in the future?
   
   +1


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

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


[GitHub] [bookkeeper] zymap merged pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
zymap merged PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454


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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454#issuecomment-1224325251

   @zymap @eolivelli this pr can be merged,
   please cherry pick to release/4.15.1


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

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


[GitHub] [bookkeeper] zymap commented on a diff in pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
zymap commented on code in PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454#discussion_r952496082


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -511,9 +511,9 @@ public void run() {
             boolean shouldForceWrite = true;
             int numReqInLastForceWrite = 0;
             long busyStartTime = System.nanoTime();
+            boolean forceWriteMarkerSent = false;

Review Comment:
   Thanks. I got it



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

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


[GitHub] [bookkeeper] zymap commented on a diff in pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
zymap commented on code in PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454#discussion_r952488118


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -511,9 +511,9 @@ public void run() {
             boolean shouldForceWrite = true;
             int numReqInLastForceWrite = 0;
             long busyStartTime = System.nanoTime();
+            boolean forceWriteMarkerSent = false;

Review Comment:
   Are you sure this is impacting the performance? Looks like the value is to make sure the marker enqueue the forcewriterequest successfully.



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

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


[GitHub] [bookkeeper] StevenLuMT commented on a diff in pull request #3454: Fix group ForceWrite not take effect with forceWriteMarkerSent in while loop of ForceWriteThread

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3454:
URL: https://github.com/apache/bookkeeper/pull/3454#discussion_r952493449


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -511,9 +511,9 @@ public void run() {
             boolean shouldForceWrite = true;
             int numReqInLastForceWrite = 0;
             long busyStartTime = System.nanoTime();
+            boolean forceWriteMarkerSent = false;

Review Comment:
   I think line:563 affects judgment @zymap 
   https://github.com/apache/bookkeeper/blob/149ecd93e75dd23447de9ea24bba68d7ef5a8e33/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java#L563



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

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