cherry-pick zstd fix + add guc for off changes#158
Merged
visill merged 14 commits intoMDB_6_25_STABLE_YEZZEYfrom Apr 30, 2025
Merged
cherry-pick zstd fix + add guc for off changes#158visill merged 14 commits intoMDB_6_25_STABLE_YEZZEYfrom
visill merged 14 commits intoMDB_6_25_STABLE_YEZZEYfrom
Conversation
gpdb uses zstd to compress workfiles, but it can't control memory allocation within zstd. Meanwhile, zstd allocates as much memory as it needs which results in ambiguous error message in a case of memory exhaustion and can make OOM killer stop gpdb processes with force. This patch forces zstd to use our custom allocator which can keep track of allocated memory. Since this zstd feature is experimental and its api can change, we also had to link zstd statically.
Smyatkin-Maxim
requested changes
Mar 17, 2025
Smyatkin-Maxim
requested changes
Apr 2, 2025
Contributor
Smyatkin-Maxim
left a comment
Smyatkin-Maxim
left a comment
There was a problem hiding this comment.
There is a leak that needs to be fixed.
IMO The perfect solution would be to make MemoryContext a member of BufFile. Then create context when we create BufFile and drop the context when we close BufFile.
The problem is that we need to pass this context somehow to custommem, BUT I see that there is a member named opaque in ZSTD_customMem. I didn't find any documentation on how to use it, but I believe you can do something like customMem.opaque = (void *)file_buf->memory_context; and use this context from customAlloc and customFree.
This way you get rid of global context (which can't be even made static), and get rid of the leak.
Smyatkin-Maxim
requested changes
Apr 16, 2025
Smyatkin-Maxim
approved these changes
Apr 21, 2025
Contributor
Smyatkin-Maxim
left a comment
Smyatkin-Maxim
left a comment
There was a problem hiding this comment.
LGTM now, feel free to squash and merge
visill
added a commit
that referenced
this pull request
Jul 2, 2025
* ADBDEV-3774 Use custom allocators for zstd (#566) gpdb uses zstd to compress workfiles, but it can't control memory allocation within zstd. Meanwhile, zstd allocates as much memory as it needs which results in ambiguous error message in a case of memory exhaustion and can make OOM killer stop gpdb processes with force. This patch forces zstd to use our custom allocator which can keep track of allocated memory. Since this zstd feature is experimental and its api can change, we also had to link zstd statically. * Add GUC --------- Co-authored-by: Dennis Kovalenko <kds@arenadata.io>
visill
added a commit
that referenced
this pull request
Jul 2, 2025
* ADBDEV-3774 Use custom allocators for zstd (#566) gpdb uses zstd to compress workfiles, but it can't control memory allocation within zstd. Meanwhile, zstd allocates as much memory as it needs which results in ambiguous error message in a case of memory exhaustion and can make OOM killer stop gpdb processes with force. This patch forces zstd to use our custom allocator which can keep track of allocated memory. Since this zstd feature is experimental and its api can change, we also had to link zstd statically. * Add GUC --------- Co-authored-by: Dennis Kovalenko <kds@arenadata.io>
reshke
pushed a commit
that referenced
this pull request
Jul 2, 2025
* ADBDEV-3774 Use custom allocators for zstd (#566) gpdb uses zstd to compress workfiles, but it can't control memory allocation within zstd. Meanwhile, zstd allocates as much memory as it needs which results in ambiguous error message in a case of memory exhaustion and can make OOM killer stop gpdb processes with force. This patch forces zstd to use our custom allocator which can keep track of allocated memory. Since this zstd feature is experimental and its api can change, we also had to link zstd statically. * Add GUC --------- Co-authored-by: Dennis Kovalenko <kds@arenadata.io>
reshke
pushed a commit
that referenced
this pull request
Jul 2, 2025
* ADBDEV-3774 Use custom allocators for zstd (#566) gpdb uses zstd to compress workfiles, but it can't control memory allocation within zstd. Meanwhile, zstd allocates as much memory as it needs which results in ambiguous error message in a case of memory exhaustion and can make OOM killer stop gpdb processes with force. This patch forces zstd to use our custom allocator which can keep track of allocated memory. Since this zstd feature is experimental and its api can change, we also had to link zstd statically. * Add GUC --------- Co-authored-by: Dennis Kovalenko <kds@arenadata.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here are some reminders before you submit the pull request
make installcheck