You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2026/03/24 16:42:23 UTC
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Ashwani Raina has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24134
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
[fs] fix partial blocks deletion case in rocksdb
This fixes a bug in rocksdb block deletion logic where only a partial
number of total blocks are successfully deleted, but the function
returns total number of blocks as 'deleted' inside the out parameter
i.e., 'deleted_block_ids'. However, the same case is handled in native
container by resizing the out parameter to correct number of blocks that
were deleted successfully, at the end of function scope.
The fix is to apply the same scoped cleanup logic for rocksdb method as
well by resizing the 'deleted_block_ids' out parameter with total number
of committed blocks. A unit test is added to verify the same.
Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
---
M src/kudu/fs/dir_manager.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
4 files changed, 119 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/24134/1
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Thu, 26 Mar 2026 13:32:29 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Wed, 01 Apr 2026 06:45:32 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 3: Verified+1 Code-Review+2
unrelated test failure:
tsan: org.apache.kudu.spark.kudu.DefaultSourceTest
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Wed, 01 Apr 2026 08:00:55 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24134/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/24134/1/src/kudu/fs/log_block_manager.cc@2128
PS1, Line 2128: // Purge the blocks that failed to delete.
> Shouldn't this say something like this: "trim to only the committed deletio
I agree, that's confusing. Done.
http://gerrit.cloudera.org:8080/#/c/24134/1/src/kudu/fs/log_block_manager.cc@2169
PS1, Line 2169: committed_count += batch_count;
> Nit: write_count isn't incremented here after the final flush succeeds, unl
Doesn't serve any purpose at present but it wouldn't hurt to keep it consistent.
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Thu, 26 Mar 2026 13:42:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager-test.cc@2896
PS2, Line 2896: constexpr int kBatchSize = 5;
: constexpr int kCommittedBlocks = kBatchSize; // first batch: blocks [0..4]
: constexpr int kUncommittedBlocks = kBatchSize; // second batch: blocks [5..9]
: constexpr int kTotalBlocks = kCommittedBlocks + kUncommittedBlocks; /
> nit: make these constexpr?
Done
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager.cc@133
PS2, Line 133: If the value is non-zero
> Any special meaning for a negative value of this flag?
Not really. Changed to uint32.
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager.cc@2126
PS2, Line 2126: siz
> nit: consider using size_t for counters like this
Done
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Tue, 31 Mar 2026 15:22:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has removed a vote on this change.
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2417/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 24 Mar 2026 17:30:39 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
[fs] fix partial blocks deletion case in rocksdb
This fixes a bug in rocksdb block deletion logic where only a partial
number of total blocks are successfully deleted, but the function
returns total number of blocks as 'deleted' inside the out parameter
i.e., 'deleted_block_ids'. However, the same case is handled in native
container by resizing the out parameter to correct number of blocks that
were deleted successfully, at the end of function scope.
The fix is to apply the same scoped cleanup logic for rocksdb method as
well by resizing the 'deleted_block_ids' out parameter with total number
of committed blocks. A unit test is added to verify the same.
Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Reviewed-on: http://gerrit.cloudera.org:8080/24134
Reviewed-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Marton Greber <gr...@gmail.com>
Tested-by: Marton Greber <gr...@gmail.com>
---
M src/kudu/fs/dir_manager.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
4 files changed, 120 insertions(+), 0 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Marton Greber: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2417/
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 24 Mar 2026 16:42:30 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2426/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 27 Mar 2026 05:57:14 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2440/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Tue, 31 Mar 2026 16:02:44 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2426/
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 27 Mar 2026 05:05:38 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24134
to look at the new patch set (#3).
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
[fs] fix partial blocks deletion case in rocksdb
This fixes a bug in rocksdb block deletion logic where only a partial
number of total blocks are successfully deleted, but the function
returns total number of blocks as 'deleted' inside the out parameter
i.e., 'deleted_block_ids'. However, the same case is handled in native
container by resizing the out parameter to correct number of blocks that
were deleted successfully, at the end of function scope.
The fix is to apply the same scoped cleanup logic for rocksdb method as
well by resizing the 'deleted_block_ids' out parameter with total number
of committed blocks. A unit test is added to verify the same.
Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
---
M src/kudu/fs/dir_manager.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
4 files changed, 120 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/24134/3
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 3:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2440/
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Tue, 31 Mar 2026 15:22:00 +0000
Gerrit-HasComments: No
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24134/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/24134/1/src/kudu/fs/log_block_manager.cc@2128
PS1, Line 2128: // Purge the blocks that failed to delete.
Shouldn't this say something like this: "trim to only the committed deletions."?
http://gerrit.cloudera.org:8080/#/c/24134/1/src/kudu/fs/log_block_manager.cc@2169
PS1, Line 2169: committed_count += batch_count;
Nit: write_count isn't incremented here after the final flush succeeds, unlike the mid-loop path. Since it's only used for test injection this is harmless, but for consistency should it be incremented here too?
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Wed, 25 Mar 2026 11:47:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Yingchun Lai, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24134
to look at the new patch set (#2).
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
[fs] fix partial blocks deletion case in rocksdb
This fixes a bug in rocksdb block deletion logic where only a partial
number of total blocks are successfully deleted, but the function
returns total number of blocks as 'deleted' inside the out parameter
i.e., 'deleted_block_ids'. However, the same case is handled in native
container by resizing the out parameter to correct number of blocks that
were deleted successfully, at the end of function scope.
The fix is to apply the same scoped cleanup logic for rocksdb method as
well by resizing the 'deleted_block_ids' out parameter with total number
of committed blocks. A unit test is added to verify the same.
Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
---
M src/kudu/fs/dir_manager.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
4 files changed, 120 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/24134/2
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [fs] fix partial blocks deletion case in rocksdb
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24134 )
Change subject: [fs] fix partial blocks deletion case in rocksdb
......................................................................
Patch Set 2: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager-test.cc@2896
PS2, Line 2896: const int kBatchSize = 5;
: const int kCommittedBlocks = kBatchSize; // first batch: blocks [0..4]
: const int kUncommittedBlocks = kBatchSize; // second batch: blocks [5..9]
: const int kTotalBlocks = kCommittedBlocks + kUncommittedBlocks; // 10
nit: make these constexpr?
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager.cc@133
PS2, Line 133: If the value is non-zero
Any special meaning for a negative value of this flag?
http://gerrit.cloudera.org:8080/#/c/24134/2/src/kudu/fs/log_block_manager.cc@2126
PS2, Line 2126: int
nit: consider using size_t for counters like this
--
To view, visit http://gerrit.cloudera.org:8080/24134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb553fbf9a88f0cdb392f926289bbdf5f18b5d29
Gerrit-Change-Number: 24134
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Mon, 30 Mar 2026 14:53:38 +0000
Gerrit-HasComments: Yes