[GitHub] merlimat commented on a change in pull request #1315: (WIP) Issue 1314: Provide a mechanism to allow high priority writes to readonly bookies

[GitBox <gi...@apache.org>]

merlimat commented on a change in pull request #1315: (WIP) Issue 1314: Provide a mechanism to allow high priority writes to readonly bookies
URL: https://github.com/apache/bookkeeper/pull/1315#discussion_r179194064
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
 ##########
 @@ -373,43 +373,49 @@ public NoWritableLedgerDirException(String errMsg) {
          *
          * @param disk Failed disk
          */
-        void diskFailed(File disk);
+        default void diskFailed(File disk) {}
 
         /**
          * Notified when the disk usage warn threshold is exceeded on the drive.
          * @param disk
          */
-        void diskAlmostFull(File disk);
+        default void diskAlmostFull(File disk) {}
 
         /**
          * This will be notified on disk detected as full.
          *
          * @param disk Filled disk
          */
-        void diskFull(File disk);
+        default void diskFull(File disk) {}
 
         /**
          * This will be notified on disk detected as writable and under warn threshold.
          *
          * @param disk Writable disk
          */
-        void diskWritable(File disk);
+        default void diskWritable(File disk) {}
 
         /**
          * This will be notified on disk detected as writable but still in warn threshold.
          *
          * @param disk Writable disk
          */
-        void diskJustWritable(File disk);
+        default void diskJustWritable(File disk) {}
 
         /**
          * This will be notified whenever all disks are detected as full.
+         *
+         * <p>Normal writes will be rejected when disks are detected as "full". High priority writes
+         * such as ledger recovery writes can go through if disks are still available.
+         *
+         * @param disksUnavailable the parameter indicates whether disks are still available for
+         *                         taking high priority writes.
          */
-        void allDisksFull();
+        default void allDisksFull(boolean disksUnavailable) {}
 
 Review comment:
   The negated logic of the parameter is a bit confusing (if `true` then it's not available).
   What about naming it something like `boolean highPriorityWritesAllowed`?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services