You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/07/01 02:36:29 UTC

[GitHub] [bookkeeper] lordcheng10 opened a new pull request, #3378: remove writeCacheRotationLock when insert writeCache

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

   ### Motivation
   remove writeCacheRotationLock when insert writeCache.
   
   We found that the write latency of bookkeeper occasionally increased suddenly, and by adding logs, we found that waiting for writeCacheRotationLock occasionally took a lot of time.
   
   By analyzing the code, I think it is not necessary to add writeCacheRotationLock when inserting data into the write cache.


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

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


[GitHub] [bookkeeper] lordcheng10 commented on pull request #3378: remove writeCacheRotationLock when insert writeCache

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

   > Please explain why this lock is not needed. this lock is used to avoid data is inserted to a writecachebeingflushed.
   
   When the entry is inserted into the write cache, if two write caches are being exchanged at this time, the entry will definitely be inserted into one of the caches anyway, because the write cache is exchanged first, then the cache is flushed and cleaned up, so the entry is inserted. Data is written to the file with the current or next flush.  @gaozhangmin 


-- 
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: issues-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 #3378: remove writeCacheRotationLock when insert writeCache

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


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -401,24 +401,7 @@ public long addEntry(ByteBuf entry) throws IOException, BookieException {
             log.debug("Add entry. {}@{}, lac = {}", ledgerId, entryId, lac);
         }
 
-        // First we try to do an optimistic locking to get access to the current write cache.
-        // This is based on the fact that the write cache is only being rotated (swapped) every 1 minute. During the
-        // rest of the time, we can have multiple thread using the optimistic lock here without interfering.
-        long stamp = writeCacheRotationLock.tryOptimisticRead();
-        boolean inserted = false;
-
-        inserted = writeCache.put(ledgerId, entryId, entry);
-        if (!writeCacheRotationLock.validate(stamp)) {
-            // The write cache was rotated while we were inserting. We need to acquire the proper read lock and repeat
-            // the operation because we might have inserted in a write cache that was already being flushed and cleared,
-            // without being sure about this last entry being flushed or not.
-            stamp = writeCacheRotationLock.readLock();
-            try {
-                inserted = writeCache.put(ledgerId, entryId, entry);
-            } finally {
-                writeCacheRotationLock.unlockRead(stamp);
-            }
-        }
+       boolean inserted = writeCache.put(ledgerId, entryId, entry);

Review Comment:
   @eolivelli @lordcheng10
   I think this lock is necessary 
   when swapWriteCache cost long time, during this time, writeCacheRotationLock.writeLock takes effect 
   we need to care this action, block writeCache's put action, prevent more write pressure on writecache



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

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


[GitHub] [bookkeeper] lordcheng10 closed pull request #3378: remove writeCacheRotationLock when insert writeCache

Posted by GitBox <gi...@apache.org>.
lordcheng10 closed pull request #3378: remove writeCacheRotationLock when insert writeCache
URL: https://github.com/apache/bookkeeper/pull/3378


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

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


[GitHub] [bookkeeper] gaozhangmin commented on pull request #3378: remove writeCacheRotationLock when insert writeCache

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

   Please explain why this lock is not needed.  @lordcheng10 


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

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


[GitHub] [bookkeeper] lordcheng10 commented on pull request #3378: remove writeCacheRotationLock when insert writeCache

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

   @eolivelli Do you have any suggestions for locking writeCacheRotationLock when writing to the cache?


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

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