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/20 05:03:11 UTC

[GitHub] [bookkeeper] StevenLuMT opened a new pull request, #3103: add async or sync for :entry location index's rocksdb write

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

   Descriptions of the changes in this PR:
   
   ### Motivation
   
   bookie timed to flush latency mainly composed of three pieces:
         1.  flush-entrylog:   it's  the latency for flushing entrylog 
         2. flush-locations-index: it's  the latency for flushing entrly location index,  use **sync** mode to flush
         3. flush-ledger-index:  it's  the latency for flushing ledger metadata index, this index(LedgerMetadataIndex) use **async** mode to flush
   
   reduce entry location index flush latency to reduce bookie's flush latency,
   so add async mode for entry location index's write: 
         1.  default sync mode: not change the original logic
         2. async mode : need update config to open, this mode is to speed up writing, this mode is the same as bookie's another index: LedgerMetadataIndex
   
   sync is different from async:
   - sync mode:  
         1.  create a batch; 
         4.  add msg to the batch
         5.  call method(batch.flush) to flush the batch
   
   - sync mode:  
         1. just call method(locationsDb.sync)  to write the data
         2. the rocksdb server will be timed to flush the data 
   
   ### Changes
   
   1. add async or sync for :location rocksdb write
   2. add switch to open this feature, default not open, use default sync 


-- 
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 #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] merlimat commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   > In order to speed up the flush, we only need to ensure that the EntryLocationIndex is successfully written under normal circumstances, regardless of machine downtime.
   
   I'm not sure I follow here. 
   
   My point is that if flushing entry logs takes 90% and RocksDB takes 10% of time (I made up these number here), then adding the risk of losing data to shave time on the 10% portion doesn't make much sense.
   
   


-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT closed pull request #3103: add async or sync for :entry location index's rocksdb write

Posted by GitBox <gi...@apache.org>.
StevenLuMT closed pull request #3103: add async or sync for :entry location index's rocksdb write
URL: https://github.com/apache/bookkeeper/pull/3103


-- 
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 #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   > I'm not sure I understand why we need to have 2 different modes here. Do both methods provide the same guarantees or not?
   
   In the case of multiple replications, when the user pursues the writing speed and does not need to guarantee the success rate of each replication, the asynchronous writing method like LedgerMetadataIndex gives the user more choices @merlimat 
   


-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

   > 
   
   ok,I understand what you mean, let me collect the time and proportion of the three parts of flush @merlimat 


-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] merlimat commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   I still cannot see a case where flushing the entry location index (ledgerId, entryId, offset) becomes the bottleneck, compared to: 
    1. Journal
    2. Entry log flush (where all the data has to be put on disk)
    
   If it is not the bottleneck, then why reduce the guarantees? (eg: if we miss the async update, we have effectively lost data in the bookie).


-- 
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 #3103: add async or sync for :entry location index's rocksdb write

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


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java:
##########
@@ -1809,6 +1810,28 @@ public ServerConfiguration setIsForceGCAllowWhenNoSpace(boolean force) {
         return this;
     }
 
+    /**
+     * Get location index sync data.
+     *
+     * @return true  - sync operate location index,
+     *         false - async operate location index.
+     */
+    public boolean getLocationIndexSyncData() {

Review Comment:
   > This option is not for every LedgerStorage. We should reflect this into the name
   
   @eolivelli good suggestion,I have rename it to getDbLedgerLocationIndexSyncEnable
   



-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   > You are on your way! I left a suggestion for the configuration entry.
   > 
   > We must also add comprehensive tests
   
   yeah, I have added two testcase: EntryLocationIndexAsyncTest/EntryLocationIndexSyncTest @eolivelli 


-- 
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 #3103: add async or sync for :entry location index's rocksdb write

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


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java:
##########
@@ -241,25 +222,27 @@ public void removeOffsetFromDeletedLedgers() throws IOException {
                     if (log.isDebugEnabled()) {
                         log.debug("Deleting index for ({}, {})", keyToDelete.getFirst(), keyToDelete.getSecond());
                     }
-                    batch.remove(keyToDelete.array);
+                    delete(batch, keyToDelete);
                     ++deletedEntriesInBatch;
                 }
 
                 if (deletedEntriesInBatch > DELETE_ENTRIES_BATCH_SIZE) {
-                    batch.flush();
-                    batch.clear();
+                    flush(batch);
                     deletedEntriesInBatch = 0;
                 }
             }
         } finally {
             try {
-                batch.flush();
-                batch.clear();
+                flush(batch);
+
+                if (deletedEntries != 0) {
+                    locationsDb.compact(firstDeletedKey, keyToDelete.array);

Review Comment:
   > are you sure that you want to compact for every deleted entry ?
   
   my mistake, I clear merged uncleaned code,thanks  @eolivelli 



-- 
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] merlimat commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   I'm not sure I understand why we need to have 2 different modes here. Do both methods provide the same guarantees or not?


-- 
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] eolivelli commented on a diff in pull request #3103: add async or sync for :entry location index's rocksdb write

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


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java:
##########
@@ -241,25 +222,27 @@ public void removeOffsetFromDeletedLedgers() throws IOException {
                     if (log.isDebugEnabled()) {
                         log.debug("Deleting index for ({}, {})", keyToDelete.getFirst(), keyToDelete.getSecond());
                     }
-                    batch.remove(keyToDelete.array);
+                    delete(batch, keyToDelete);
                     ++deletedEntriesInBatch;
                 }
 
                 if (deletedEntriesInBatch > DELETE_ENTRIES_BATCH_SIZE) {
-                    batch.flush();
-                    batch.clear();
+                    flush(batch);
                     deletedEntriesInBatch = 0;
                 }
             }
         } finally {
             try {
-                batch.flush();
-                batch.clear();
+                flush(batch);
+
+                if (deletedEntries != 0) {
+                    locationsDb.compact(firstDeletedKey, keyToDelete.array);

Review Comment:
   are you sure that you want to compact for every deleted entry ?



-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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: 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 #3103: add async or sync for :entry location index's rocksdb write

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


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java:
##########
@@ -241,25 +222,27 @@ public void removeOffsetFromDeletedLedgers() throws IOException {
                     if (log.isDebugEnabled()) {
                         log.debug("Deleting index for ({}, {})", keyToDelete.getFirst(), keyToDelete.getSecond());
                     }
-                    batch.remove(keyToDelete.array);
+                    delete(batch, keyToDelete);
                     ++deletedEntriesInBatch;
                 }
 
                 if (deletedEntriesInBatch > DELETE_ENTRIES_BATCH_SIZE) {
-                    batch.flush();
-                    batch.clear();
+                    flush(batch);
                     deletedEntriesInBatch = 0;
                 }
             }
         } finally {
             try {
-                batch.flush();
-                batch.clear();
+                flush(batch);
+
+                if (deletedEntries != 0) {
+                    locationsDb.compact(firstDeletedKey, keyToDelete.array);

Review Comment:
   > are you sure that you want to compact for every deleted entry ?
   
   my mistake, I clear merged uncleaned code



-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

   > I still cannot see a case where flushing the entry location index (ledgerId, entryId, offset) becomes the bottleneck, compared to:
   > 
   > 1. Journal
   > 2. Entry log flush (where all the data has to be put on disk)
   > 
   > If it is not the bottleneck, then why reduce the guarantees? (eg: if we miss the async update, we have effectively lost data in the bookie).
   
   yes, in the step : entrylog flush 
   In order to speed up the flush, we only need to ensure that the EntryLocationIndex is successfully written under normal circumstances, regardless of machine downtime.
   Like LedgerMetadataIndex, it also uses an asynchronous method to write rocksdb @merlimat 


-- 
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 pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

   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] StevenLuMT commented on pull request #3103: add async or sync for :entry location index's rocksdb write

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

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

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