Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: crash during getting info about replication #4328

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

BorysTheDev
Copy link
Contributor

@BorysTheDev BorysTheDev commented Dec 17, 2024

problem: we try to get the memory info during replication start, but the snapshot object doesn't exist yet

adiholden
adiholden previously approved these changes Dec 17, 2024
@@ -1333,6 +1333,8 @@ size_t RdbSaver::Impl::GetTotalBuffersSize() const {

auto cb = [this, &channel_bytes, &serializer_bytes](ShardId sid) {
auto& snapshot = shard_snapshots_[sid];
if (!snapshot)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BorysTheDev please add a comment explaining how come RdbSave::Impl with shard_snapshots_ exist but they are not initialized yed.

@@ -1355,6 +1357,8 @@ RdbSaver::SnapshotStats RdbSaver::Impl::GetCurrentSnapshotProgress() const {

auto cb = [this, &results](ShardId sid) {
auto& snapshot = shard_snapshots_[sid];
if (!snapshot)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@BorysTheDev BorysTheDev force-pushed the crash_INFO_cmd_druing_replication branch from 61a5a3b to be70b39 Compare December 17, 2024 13:42
@BorysTheDev BorysTheDev requested a review from romange December 17, 2024 13:43
@BorysTheDev BorysTheDev merged commit 15b293a into main Dec 18, 2024
9 checks passed
@BorysTheDev BorysTheDev deleted the crash_INFO_cmd_druing_replication branch December 18, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants