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/06/17 01:57:46 UTC
[GitHub] [bookkeeper] StevenLuMT opened a new pull request, #3342: Bookie cache optimization
StevenLuMT opened a new pull request, #3342:
URL: https://github.com/apache/bookkeeper/pull/3342
Descriptions of the changes in this PR:
### Motivation
When flushing, trigger flush to exchange writeCache and flushBeingCache
flushBeingCache will be cleaned up immediately after flushed
this logic causes the cache capacity to fluctuate greatly
### Changes
This optimization mainly solves this point and realizes the logic:
1. When flushBeingCache is flushed to disk, it is not deleted immediately, but dumped to lastFlushCache
2. After the dump is completed, clear the flushBeingCache again
Eventually: the cache has a fixed cache duration
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160314272
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161035418
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161209245
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161378588
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1162580028
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160351029
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] eolivelli commented on pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1162939806
@merlimat @hangc0276 do you have other comments ?
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r899826615
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -161,7 +164,7 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
this.writeCacheMaxSize = writeCacheSize;
this.writeCache = new WriteCache(allocator, writeCacheMaxSize / 2);
this.writeCacheBeingFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
-
+ this.writeCacheLastFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
Review Comment:
@merlimat
thanks for your suggestion，I have update the init code
<img width="1248" alt="image" src="https://user-images.githubusercontent.com/42990025/174589904-f7900999-cc3e-4921-9618-5ba182f9596a.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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] dlg99 commented on a diff in pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
dlg99 commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r902850722
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -191,8 +204,10 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
dbLedgerStorageStats = new DbLedgerStorageStats(
ledgerDirStatsLogger,
- () -> writeCache.size() + writeCacheBeingFlushed.size(),
- () -> writeCache.count() + writeCacheBeingFlushed.count(),
+ () -> this.isWriteCacheFixedLengthEnabled ? writeCache.size() + writeCacheBeingFlushed.size()
+ + writeCacheLastFlushed.size() : writeCache.size() + writeCacheBeingFlushed.size(),
+ () -> this.isWriteCacheFixedLengthEnabled ? writeCache.count() + writeCacheBeingFlushed.count()
+ + writeCacheLastFlushed.count() : writeCache.count() + writeCacheBeingFlushed.count(),
Review Comment:
```suggestion
this.isWriteCacheFixedLengthEnabled ? () -> writeCache.size() + writeCacheBeingFlushed.size()
+ writeCacheLastFlushed.size() : () -> writeCache.size() + writeCacheBeingFlushed.size(),
this.isWriteCacheFixedLengthEnabled ? () -> writeCache.count() + writeCacheBeingFlushed.count()
+ writeCacheLastFlushed.count() : () -> writeCache.count() + writeCacheBeingFlushed.count(),
```
in this case you don't have to check for isWriteCacheFixedLengthEnabled / isWriteCacheFixedLengthEnabled inside lambdas, these won't change dynamically as I understand
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -793,6 +843,13 @@ public void checkpoint(Checkpoint checkpoint) throws IOException {
}
}
+ private void cloneFlushedCache2LastFlushCache(WriteCache writeCacheBeingFlushed, WriteCache writeCacheLastFlushed) {
+ writeCacheLastFlushed.clear();
+ writeCacheBeingFlushed.forEach((ledgerId, entryId, data) -> {
Review Comment:
this looks like an expensive operation.
Is there any reason to not swap two caches like in `swapWriteCache()`?
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161309440
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r899826615
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -161,7 +164,7 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
this.writeCacheMaxSize = writeCacheSize;
this.writeCache = new WriteCache(allocator, writeCacheMaxSize / 2);
this.writeCacheBeingFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
-
+ this.writeCacheLastFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
Review Comment:
@merlimat
writeCacheBeingFlushe and writeCacheLastFlushed almost only one of the two exists at the same time
so I think this just 100% memory for the write cache.
<img width="1420" alt="image" src="https://user-images.githubusercontent.com/42990025/174243987-33261c58-0289-47f3-af5d-c7b0768228b3.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: 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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1158571353
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161440373
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
merlimat commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1163323820
@StevenLuMT Since you have now reduce the sizes to 1/3 each, what will happen is that the writes are blocked when we reach 66% of memory instead of 100%.
I don't really understand why is it a problem for the write cache to fluctuate in size.
The write cache job is to make sure we can decouple write path from the flushing. The read cache job is to provide high hit-rate.
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160648766
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r901550399
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -161,7 +164,7 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
this.writeCacheMaxSize = writeCacheSize;
this.writeCache = new WriteCache(allocator, writeCacheMaxSize / 2);
this.writeCacheBeingFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
-
+ this.writeCacheLastFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
Review Comment:
>
@merlimat
thanks for your suggestion，I have update the init code
<img width="1248" alt="image" src="https://user-images.githubusercontent.com/42990025/174589904-f7900999-cc3e-4921-9618-5ba182f9596a.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: 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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160355168
> the change makes sense to me, do you have some numbers ?
yes, I have added some test data in Descriptions of the pr @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 pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161122040
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161184017
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160376727
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160580336
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1159918454
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160356914
> the change makes sense to me, do you have some numbers ?
yes, I have added some test data in Descriptions of the pr @eolivelli ,help me review it ,thanks
--
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] hangc0276 commented on pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1193449764
@StevenLuMT Do we still need this cache optimization?
--
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 pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1158590166
the change makes sense to me, do you have some numbers ?
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160422182
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] hangc0276 commented on pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1165241800
> > Since you have now reduce the sizes to 1/3 each, what will happen is that the writes are blocked when we reach 66% of memory instead of 100%.
> > I don't really understand why is it a problem for the write cache to fluctuate in size. The write cache job is to make sure we can decouple write path from the flushing. The read cache job is to provide high hit-rate.
>
> I think this is an option when more real-time data needs to be persisted in writecache, instead of triggering read disk to load into readcache @merlimat
@StevenLuMT I agree with Matteo. I do not really understand the motivation of this PR. I have the following concerns.
- From your test data, you compared the cached data size, it certainly will be more than the old one. But the size of cached data is not our goal, but the bookie cache hit rate. We should leverage the gain and the cost. The gain is the hit rate, and the cost is the extra memory used.
- What we are concerned about is whether this change is valuable, not just adding an option to turn it on or off.
- We use 1/3 memory cache to cache the last flushed data. Those data contain all the newly written data, it may not improve the write cache hit rate a lot due to those cached data without purpose. This is why we evict the flushed data from OS PageCache once it is written into the journal disk. If we really need to cache the last flushed data into memory, I prefer to use OS PageCache.
- I'm not sure whether you use BookKeeper based on Pulsar. If you use Pulsar, I prefer to tunning Pulsar broker cache instead of BookKeeper write cache. The Pulsar broker cache and BookKeeper read cache are driven by reading. It will have higher hit rate.
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1162944449
> The motivation is to increase the write cache hit rate, right? The cost is that it will use more than 50% write cache memory. I suggestion to tune the cache hit rate on the Pulsar broker side instead of the bookie side.
>
> Otherwise, the write cache holds all the recent data, and the hit rate maybe not be higher than the read cache. I prefer to tune the broker's cache and the bookie's read cache.
I add a switch ,if user can enable open this feature @hangc0276
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1194429447
> @StevenLuMT Do we still need this cache optimization?
Close it for now, I'll bring it up later when I have better ideas @hangc0276
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161154878
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161271560
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1163814056
> Since you have now reduce the sizes to 1/3 each, what will happen is that the writes are blocked when we reach 66% of memory instead of 100%.
>
> I don't really understand why is it a problem for the write cache to fluctuate in size. The write cache job is to make sure we can decouple write path from the flushing. The read cache job is to provide high hit-rate.
I think this is an option when more real-time data needs to be persisted in writecache,instead of triggering read disk to load into readcache @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 a diff in pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r903195614
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -191,8 +204,10 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
dbLedgerStorageStats = new DbLedgerStorageStats(
ledgerDirStatsLogger,
- () -> writeCache.size() + writeCacheBeingFlushed.size(),
- () -> writeCache.count() + writeCacheBeingFlushed.count(),
+ () -> this.isWriteCacheFixedLengthEnabled ? writeCache.size() + writeCacheBeingFlushed.size()
+ + writeCacheLastFlushed.size() : writeCache.size() + writeCacheBeingFlushed.size(),
+ () -> this.isWriteCacheFixedLengthEnabled ? writeCache.count() + writeCacheBeingFlushed.count()
+ + writeCacheLastFlushed.count() : writeCache.count() + writeCacheBeingFlushed.count(),
Review Comment:
good suggestion
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1158392454
@dlg99 @eolivelli @pkumar-singh @zymap @hangc0276 @lordcheng10
If you have time, please help me review it, thank you.
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1160613312
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r899826615
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -161,7 +164,7 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
this.writeCacheMaxSize = writeCacheSize;
this.writeCache = new WriteCache(allocator, writeCacheMaxSize / 2);
this.writeCacheBeingFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
-
+ this.writeCacheLastFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
Review Comment:
@merlimat
writeCacheBeingFlushe and writeCacheLastFlushed almost only one of the two exists at the same time
so I think this just 100% memory for the write cache.
<img width="1248" alt="image" src="https://user-images.githubusercontent.com/42990025/174243987-33261c58-0289-47f3-af5d-c7b0768228b3.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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] merlimat commented on a diff in pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r899818425
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -161,7 +164,7 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
this.writeCacheMaxSize = writeCacheSize;
this.writeCache = new WriteCache(allocator, writeCacheMaxSize / 2);
this.writeCacheBeingFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
-
+ this.writeCacheLastFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
Review Comment:
This will make us use 150% of the configured memory for 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] StevenLuMT commented on pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161585516
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r903198790
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -793,6 +843,13 @@ public void checkpoint(Checkpoint checkpoint) throws IOException {
}
}
+ private void cloneFlushedCache2LastFlushCache(WriteCache writeCacheBeingFlushed, WriteCache writeCacheLastFlushed) {
+ writeCacheLastFlushed.clear();
+ writeCacheBeingFlushed.forEach((ledgerId, entryId, data) -> {
Review Comment:
> this looks like an expensive operation. Is there any reason to not swap two caches like in `swapWriteCache()`?
I think this way can make your data more safe
--
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 a diff in pull request #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r900242420
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -161,7 +164,7 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager le
this.writeCacheMaxSize = writeCacheSize;
this.writeCache = new WriteCache(allocator, writeCacheMaxSize / 2);
this.writeCacheBeingFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
-
+ this.writeCacheLastFlushed = new WriteCache(allocator, writeCacheMaxSize / 2);
Review Comment:
The "flush" to disk is not instantaneous. In fact, it can take a very long time and during that time all 3 segments will be filled up during that time.
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1158458773
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#issuecomment-1161242583
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on code in PR #3342:
URL: https://github.com/apache/bookkeeper/pull/3342#discussion_r903198790
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -793,6 +843,13 @@ public void checkpoint(Checkpoint checkpoint) throws IOException {
}
}
+ private void cloneFlushedCache2LastFlushCache(WriteCache writeCacheBeingFlushed, WriteCache writeCacheLastFlushed) {
+ writeCacheLastFlushed.clear();
+ writeCacheBeingFlushed.forEach((ledgerId, entryId, data) -> {
Review Comment:
> this looks like an expensive operation. Is there any reason to not swap two caches like in `swapWriteCache()`?
good suggestion,I optimized this function
--
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 #3342: Bookie cache optimization
Posted by GitBox <gi...@apache.org>.
StevenLuMT closed pull request #3342: Bookie cache optimization
URL: https://github.com/apache/bookkeeper/pull/3342
--
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