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/09/30 06:37:58 UTC

[GitHub] [bookkeeper] gaozhangmin opened a new pull request, #3509: Add new api resumeCompaction and suspendCompaction

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

   ### Motivation
   Compaction would bring high disk io util, It would better allowing to control suspending and resuming compaction through api
   
   ### Changes
   Add two apis to control resuming and suspending compaction.
   `/api/v1/bookie/compaction/suspend`
   `/api/v1/bookie/compaction/resume`
   


-- 
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 #3509: Add new api resumeCompaction and suspendCompaction

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

   ci fail,have a look the error info
   <img width="1321" alt="image" src="https://user-images.githubusercontent.com/42990025/193214837-6b13784b-7b54-45f5-b2ae-2736e302b373.png">
   


-- 
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] hangc0276 commented on a diff in pull request #3509: Add new api resumeCompaction and suspendCompaction

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


##########
bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java:
##########
@@ -51,6 +51,8 @@
     public static final String BOOKIE_STATE_READONLY        = "/api/v1/bookie/state/readonly";
     public static final String BOOKIE_IS_READY              = "/api/v1/bookie/is_ready";
     public static final String BOOKIE_INFO                  = "/api/v1/bookie/info";
+    public static final String SUSPEND_COMPACTION           = "/api/v1/bookie/compaction/suspend";
+    public static final String RESUME_COMPACTION            = "/api/v1/bookie/compaction/resume";

Review Comment:
   I prefer to use `gc` endpoint instead of `compaction`, It will introduce more confusion.



-- 
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] gaozhangmin commented on pull request #3509: Add new api resumeCompaction and suspendCompaction

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

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

To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org

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


[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3509: Add new api resumeCompaction and suspendCompaction

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


##########
bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java:
##########
@@ -51,6 +51,8 @@
     public static final String BOOKIE_STATE_READONLY        = "/api/v1/bookie/state/readonly";
     public static final String BOOKIE_IS_READY              = "/api/v1/bookie/is_ready";
     public static final String BOOKIE_INFO                  = "/api/v1/bookie/info";
+    public static final String SUSPEND_COMPACTION           = "/api/v1/bookie/compaction/suspend";

Review Comment:
   +1 for @dlg99. I prefer to use `gc` instead of `compaction` endpoint. It won't introduce break changes.



-- 
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] gaozhangmin commented on pull request #3509: Add new api resumeCompaction and suspendCompaction

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

   @hangc0276  PTAL again @dlg99 


-- 
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] hangc0276 merged pull request #3509: Add new api resumeCompaction and suspendCompaction

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


-- 
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] gaozhangmin commented on pull request #3509: Add new api resumeCompaction and suspendCompaction

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

   @dlg99  PTAL again, thx


-- 
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] gaozhangmin commented on pull request #3509: Add new api resumeCompaction and suspendCompaction

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

   api docs added


-- 
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] gaozhangmin commented on a diff in pull request #3509: Add new api resumeCompaction and suspendCompaction

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


##########
bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java:
##########
@@ -51,6 +51,8 @@
     public static final String BOOKIE_STATE_READONLY        = "/api/v1/bookie/state/readonly";
     public static final String BOOKIE_IS_READY              = "/api/v1/bookie/is_ready";
     public static final String BOOKIE_INFO                  = "/api/v1/bookie/info";
+    public static final String SUSPEND_COMPACTION           = "/api/v1/bookie/compaction/suspend";

Review Comment:
   Bookkeeper GC contains two parts, 1. GC ledgers and entry log. 2. compaction entry log files. 
   I don't think  we should combine the two apis together.
   `/gc` is to trigger 1 and 2, while the new api is to control part 2.



##########
bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java:
##########
@@ -51,6 +51,8 @@
     public static final String BOOKIE_STATE_READONLY        = "/api/v1/bookie/state/readonly";
     public static final String BOOKIE_IS_READY              = "/api/v1/bookie/is_ready";
     public static final String BOOKIE_INFO                  = "/api/v1/bookie/info";
+    public static final String SUSPEND_COMPACTION           = "/api/v1/bookie/compaction/suspend";

Review Comment:
   Bookkeeper GC contains two parts, 1. GC ledgers and entry log. 2. compaction entry log files. 
   I don't think  we should combine the two apis together.
   `/gc` is to trigger 1 and 2, while the new api is to control part 2.
   @dlg99 



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