Re: [PR] #2974: Add hash-based journal selection for improved ledger distribution [bookkeeper]

["jprieto-temporal (via GitHub)" <gi...@apache.org>]

jprieto-temporal commented on code in PR #4709:
URL: https://github.com/apache/bookkeeper/pull/4709#discussion_r2754924250


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java:
##########
@@ -940,8 +942,30 @@ LedgerDescriptor getLedgerForEntry(ByteBuf entry, final byte[] masterKey)
         return handles.getHandle(ledgerId, masterKey, false);
     }
 
+    /**
+     * Constant for Fibonacci hashing. Multiplying by this constant adds randomness, breaking
+     * patterns in ledger IDs that would otherwise cause uneven journal distribution.
+     *
+     * <p>This is the same constant used as {@code GOLDEN_GAMMA} in
+     * {@link java.util.concurrent.ThreadLocalRandom} and {@link java.util.SplittableRandom}
+     * for seed mixing.
+     */
+    private static final long FIBONACCI_HASH_CONSTANT = 0x9E3779B97F4A7C15L;
+
+    /**
+     * Returns the journal to use for the given ledger.
+     *
+     * <p>When hash-based selection is enabled, uses Fibonacci hashing to distribute ledgers
+     * across journals. The xor with the right-shifted value folds the high 32 bits into
+     * the low 32 bits, adding more mixing prior to the modulo.
+     */
     private Journal getJournal(long ledgerId) {
-        return journals.get(MathUtils.signSafeMod(ledgerId, journals.size()));
+        long index = ledgerId;
+        if (journalHashBasedSelection) {
+            index = ledgerId * FIBONACCI_HASH_CONSTANT;

Review Comment:
   Thank you for your review. I added the tests and factored out the static method.



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