DBZ-7117 Prevent OOM in Debezium Redis sink #61
Conversation
| totalProcessed += bufferSize; | ||
| LOGGER.debug("Total Processed Records: {}", totalProcessed); |
There was a problem hiding this comment.
If maximumMemory is set to 0, I assume we are not doing any memory check, and is there a case where totalProcessed here would just continue to grow until it overflows potentially? If so, I wonder if it would be better to avoid incrementing and simply just return true only?
There was a problem hiding this comment.
This is correct is maximumMemory we are not doing any checks. The totalProcessed is long so I dobt that Debezium will ever process 9,223,372,036,854,775,807 records without restart
| (estimatedUsedMemory >= maximumMemory), humanReadableSize(usedMemory), humanReadableSize(maximumMemory), | ||
| humanReadableSize(accumulatedMemory), humanReadableSize(estimatedUsedMemory), | ||
| humanReadableSize(extraMemory), bufferSize, totalProcessed + bufferSize); | ||
| totalProcessed += bufferSize; |
There was a problem hiding this comment.
Same as above, could this eventually overflow?
There was a problem hiding this comment.
Same answer as above
| } | ||
|
|
||
| long extimatedBatchSize = extraMemory * bufferFillRate; | ||
| long usedMemory = memoryTuple.getItem1(); |
There was a problem hiding this comment.
In the memoryTuple() function, the first element stored here could technically be null if the key for this element is not found and could result in a NullPointerException. Similar to how you guarantee there is a non-null value for item2, you should do the same for item1.
| /** | ||
| * Formats a raw file size value into a human-readable string with appropriate units (B, KB, MB, GB). | ||
| * | ||
| * @param size The size value to be formatted. | ||
| * @return A human-readable string representing the file size with units (B, KB, MB, GB). | ||
| */ | ||
| public static String humanReadableSize(Long size) { | ||
| if (size == null) { | ||
| return "Not configured"; | ||
| } | ||
|
|
||
| final String[] units = { "B", "KB", "MB", "GB" }; | ||
| int unitIndex = 0; | ||
|
|
||
| double sizeInUnit = size; | ||
|
|
||
| if (size == 0) { | ||
| return "0 B"; | ||
| } | ||
|
|
||
| while (sizeInUnit >= 1024 && unitIndex < units.length - 1) { | ||
| sizeInUnit /= 1024.0; | ||
| unitIndex++; | ||
| } | ||
|
|
||
| return String.format("%.2f %s", sizeInUnit, units[unitIndex]); | ||
| } |
There was a problem hiding this comment.
I'd suggest creating a follow-up Jira and consider moving this to debezium-core inside either the io.debezium.util.NumberConversions class or io.debezium.util.Strings utility classes as I think this could be helpful for any connector that may want to log similar bits in the future, perhaps with a variant that uses the `"%.2f %s" format by default and another that takes a format string that allows customization.
There was a problem hiding this comment.
I'd suggest to name this with a verb, something like:
- formatReadableSize
- getSizeInHumanReadableFormat
- convertSizeToReadableString
- getFormattedSizeString
There was a problem hiding this comment.
Changing to getSizeInHumanReadableFormat. I will create a new Jira for that. This fix will need to go into different repository
…ttps://github.com/eizners/debezium-server into DBZ-7117-prevent-oom-in-debezium-redis-sink-stash
eizners
force-pushed
the
DBZ-7117-prevent-oom-in-debezium-redis-sink-stash
branch
from
2dec7de to
4592876
Compare
…ttps://github.com/eizners/debezium-server into DBZ-7117-prevent-oom-in-debezium-redis-sink-stash
No description provided.