Skip to content

Conversation

@suiyuzeng
Copy link

Motivation

fix issue 2806 OOM as 1 million ledgers per entry log

Changes

1.Add two config entryLogLedgerMapConcurrency and flushEntrySortBufferInitSize, to avoid allocating a large contiguous memory.
2.Reduce memory usage of EntryLogMetadata of entry logs in garbage collector. If the retention time of all the entry is the same, we do not need to extract the entry logs in retention time to load meta to memory. In org.apache.bookkeeper.bookie.GarbageCollectorThread#extractMetaFromEntryLogs, if the entry log in retention time, do not extract the entry log.

@Vanlightly
Copy link
Contributor

Regarding garbage collection, retention is not determined by BookKeeper but whoever controls the ledger metadata (Pulsar for example). I don't think using a configured time period is the way to address this. The issue is that we try to store all metadata in memory, but given that each entry log has a ledger map, we can perform a scan process where we incrementally perform GC without all metadata in memory.

@suiyuzeng
Copy link
Author

suiyuzeng commented Nov 18, 2021

Regarding garbage collection, retention is not determined by BookKeeper but whoever controls the ledger metadata (Pulsar for example). I don't think using a configured time period is the way to address this. The issue is that we try to store all metadata in memory, but given that each entry log has a ledger map, we can perform a scan process where we incrementally perform GC without all metadata in memory.

Thanks for your suggestion.
I meet the issue in puslar for iot. There are millions topics which has the retention time. It works well in this scene. But it may be not common way to the problem like this. A scan process is better for this. I will fix the issue in this way.

@suiyuzeng
Copy link
Author

If the count of ledger is not veray large but the entry log count is very large, the memory of the ledger map will not be very large and it will lead to io read, especially for extractEntryLogMetadataByScanning. The current way is better for this. How about reserving two way and adding a config to choose?
@Vanlightly

@Vanlightly
Copy link
Contributor

@suiyuzeng it could be a way forward. I think depending on the result we may be happy with just using the scan method. So at first make it configurable which will allow us to evaluate both methods under similar conditions.

@suiyuzeng
Copy link
Author

ok

@suiyuzeng
Copy link
Author

@suiyuzeng it could be a way forward. I think depending on the result we may be happy with just using the scan method. So at first make it configurable which will allow us to evaluate both methods under similar conditions.

@Vanlightly hi, I find #1949 merged recently fix the issue when i develop the scan way. It store the meta into rocksdb. This way reduce the memory and just read the meta from the entrylog for one times. Do we need the scan way any more?

@StevenLuMT
Copy link
Member

if #1949 fix the issue, please know it @hangc0276 @Vanlightly
@suiyuzeng please close your pr, thanks

@suiyuzeng suiyuzeng closed this Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants