etcdutl: Fix snapshot restore memory alloc issue #17277
Merged
Conversation
This file contains 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
|
Hi @ivanvc. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ivanvc
force-pushed
the
fixes-etcdutl-memory-allocation-issue
branch
from
0f9a53c
to
d5f4b3e
Compare
|
/ok-to-test |
ahrtr
reviewed
Feb 9, 2024
ahrtr
reviewed
Feb 9, 2024
ivanvc
force-pushed
the
fixes-etcdutl-memory-allocation-issue
branch
2 times, most recently
from
542c737
to
11d8d88
Compare
ahrtr
reviewed
Feb 11, 2024
ivanvc
force-pushed
the
fixes-etcdutl-memory-allocation-issue
branch
from
11d8d88
to
362a524
Compare
ivanvc
force-pushed
the
fixes-etcdutl-memory-allocation-issue
branch
from
362a524
to
cd9dfaf
Compare
ahrtr
reviewed
Feb 13, 2024
server/storage/backend/backend.go
Outdated
| @@ -151,11 +151,13 @@ type BackendConfig struct { | |||
| Hooks Hooks | |||
| } | |||
|
|
|||
| type BackendConfigFunc func(*BackendConfig) | |||
There was a problem hiding this comment.
Suggested change
| type BackendConfigFunc func(*BackendConfig) | |
| type BackendConfigOption func(*BackendConfig) |
server/storage/backend/backend.go
Outdated
| } | ||
| } | ||
|
|
||
| func NewDefaultBackend(lg *zap.Logger, path string, bcfns ...BackendConfigFunc) Backend { |
There was a problem hiding this comment.
Suggested change
| func NewDefaultBackend(lg *zap.Logger, path string, bcfns ...BackendConfigFunc) Backend { | |
| func NewDefaultBackend(lg *zap.Logger, path string, opts ...BackendConfigOption) Backend { |
server/config/config.go
Outdated
| @@ -119,6 +119,7 @@ type ServerConfig struct { | |||
| CompactionBatchLimit int | |||
| CompactionSleepInterval time.Duration | |||
| QuotaBackendBytes int64 | |||
| InitialMmapSize uint64 | |||
There was a problem hiding this comment.
I don't think so, removed.
Accept a third argument for NewDefaultBackend for overrides to the BackendConfig. Add a new function, WithMmapSize, which modifies the backend config to provide a custom InitiamMmapSize. Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc
force-pushed
the
fixes-etcdutl-memory-allocation-issue
branch
from
cd9dfaf
to
4f66aa9
Compare
When running the snapshot command, allow receiving an initial memory map allocation for the database, avoiding future memory allocation issues. Co-authored-by: Benjamin Wang <benjamin.wang@broadcom.com> Co-authored-by: Fatih USTA <fatihusta86@gmail.com> Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc
force-pushed
the
fixes-etcdutl-memory-allocation-issue
branch
from
4f66aa9
to
be28833
Compare
serathius
approved these changes
Feb 14, 2024
ivanvc
added a commit
to ivanvc/etcd
that referenced
this pull request
May 10, 2024
Accept a third argument for NewDefaultBackend for overrides to the BackendConfig. Add a new function, WithMmapSize, which modifies the backend config to provide a custom InitiamMmapSize. Backports commit: d69adf4 / PR: etcd-io#17277 Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc
added a commit
to ivanvc/etcd
that referenced
this pull request
May 10, 2024
When running the snapshot command, allow receiving an initial memory map allocation for the database, avoiding future memory allocation issues. Backports commit: be28833 / PR: etcd-io#17277 Co-authored-by: Fatih USTA <fatihusta86@gmail.com> Signed-off-by: Ivan Valdes <ivan@vald.es>
This was referenced May 10, 2024
aneesh1
pushed a commit
to DataDog/etcd
that referenced
this pull request
Sep 25, 2024
When running the snapshot command, allow receiving an initial memory map allocation for the database, avoiding future memory allocation issues. Backports commit: be28833 / PR: etcd-io#17277 Co-authored-by: Fatih USTA <fatihusta86@gmail.com> Signed-off-by: Ivan Valdes <ivan@vald.es>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When running the snapshot command, allow receiving an initial memory map allocation for the database, avoiding future memory allocation issues.
As discussed in the fortnightly etc triage meeting, I'm continuing the work from #16055 by squashing the commits and rebasing @fatihusta's original branch with main.
Fixes #16052.
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.