You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mahesh Reddy (Code Review)" <ge...@cloudera.org> on 2024/05/02 20:47:36 UTC
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Mahesh Reddy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21390
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
2 files changed, 139 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/1
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 10:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.h
File src/kudu/cfile/block_cache.h:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.h@122
PS9, Line 122: explicit BlockCache(size_t capacity);
:
: BlockCache(size_t probationary_segment_capacity,
: size_t protected_segment_capacity,
: size_t lookups);
> nit: why not to introduce BlockCacheParameters to keep the signature for th
These constructors aren't even used at the moment. If I introduce a struct named 'BlockCacheParameters' with fields for all the parameters, it would look similar to BlockCache(). That begs the question of why these constructors are necessary in the first place. They would only be used if a singleton isn't used for the block cache, but currently all instances of the block cache use a singleton which uses the private constructor BlockCache(). Until there's a use for them, I think it makes sense to remove them. What do you think?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@49
PS9, Line 49: TAG_FLAG(block_cache_eviction_policy, experimental);
:
: DEFINE_double(block_cache_protected_segment_percentage, 0.8,
: "Percentage of 'block_cache_capacity_mb' that's the protected segment's capacity. "
: "Ex. For the default parameters of 0.8 for this flag and 512 MB for "
: "'block_cache_capacity_mb', the protected segment will be 0.8 * 512 = 410 MB."
: "Must be >= 0 and <= 1.");
: TAG_FLAG(block_cache_protected_segment_percentage, experimental);
> From the usability standpoint, maybe it's more better to introduce (total_c
I agree adding a percentage parameter makes more sense than specifying both the segment capacities on their own. A total capacity parameter already exists in `block_cache_capacity-mb' so I'll just remove both parameters for the segment_capacity and just add one for the percentage.
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@93
PS9, Line 93: class MetricEntity;
:
: namespace cfile {
:
: namespace {
:
: Cache* CreateCache(int64_t capacity) {
: const auto& mem_type = BlockCache::GetConfiguredCacheMemoryTypeOrDie();
: switch (mem_type) {
: case Cache::MemoryType::DRAM:
: return NewCache<Cache::EvictionPolicy::LRU, Cache::MemoryType::DRAM>(
: capacity, "block_cache");
: case Cache::MemoryType::NVM:
:
> Why not to turn this helper function into a static BlockCache::Create() fun
I guess a private static function would accomplish the same thing. I don't see any clear benefit from either at first glance so I don't see the point of changing it at the moment. Is there a reason in this case that a private static method would be preferred to an anonymous namespace? A private static function would involve defining it in the header file too which is unnecessary in this case.
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@122
PS9, Line 122: d
> nit for here and below in this file: consider adding corresponding 'using'
Done
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@123
PS9, Line 123: LOG(FATAL) << "unsupported SLRU cache memory type: " << mem_type;
: return nullptr;
: }
> Why to introduce this if there is already ToUpperCase() in src/kudu/util/st
It's used several times in this file. IMO it's cleaner to make this function that uses ToUpperCase() and to have each function call just be one line rather than declaring a new temp string each time ToUpperCase() is used which would be two lines.
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/dense_node-itest.cc
File src/kudu/integration-tests/dense_node-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/dense_node-itest.cc@174
PS9, Line 174: opts.extra_master_flags.emplace_back(Substitute("--block_manager=$0", FLAGS_block_manager));
: opts.extra_tserver_flags.emplace_back(Substitute("--block_manager=$0", FLAGS_block_manager));
:
: FLAGS_block_cache_eviction_policy = std::get<2>(setup);
: opts.extra_master_flags.emplace_back(Substitute("--block_cache_eviction_policy=$0",
> By doing this, we are removing existing test coverage for the LRU block cac
Done
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/full_stack-insert-scan-test.cc@147
PS9, Line 147:
> Shouldn't this become a parameter of the corresponding test scenarios inste
Done
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/heavy-update-compaction-itest.cc
File src/kudu/integration-tests/heavy-update-compaction-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/heavy-update-compaction-itest.cc@119
PS9, Line 119: // Encourage frequent flushes.
> Shouldn't this become a parameter for the scenarios of this test suite inst
Done
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/tserver/tablet_server-stress-test.cc
File src/kudu/tserver/tablet_server-stress-test.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/tserver/tablet_server-stress-test.cc@80
PS9, Line 80: FLAGS_enable_maintenance_manager = true;
> Shouldn't this become a parameter for the scenarios of this test suite inst
Done
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 10
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 23:20:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 10:
Build Aborted
http://jenkins.kudu.apache.org/job/pre_commit/421/ : ABORTED
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 10
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 23:21:24 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 13: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/431/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 13
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 22:13:14 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 11: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/422/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 11
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Aug 2024 00:27:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 9:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.h
File src/kudu/cfile/block_cache.h:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.h@122
PS9, Line 122: explicit BlockCache(size_t capacity);
:
: BlockCache(size_t probationary_segment_capacity,
: size_t protected_segment_capacity,
: size_t lookups);
nit: why not to introduce BlockCacheParameters to keep the signature for the constructor uniform? As an extra, one might use the Builder pattern to create an instance of BlockCacheParameters, if necessary.
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@49
PS9, Line 49:
: DEFINE_int64(block_cache_probationary_segment_capacity_mib, 102,
: "Capacity of probationary segment of block cache in MiB");
: TAG_FLAG(block_cache_probationary_segment_capacity_mib, experimental);
:
: DEFINE_int64(block_cache_protected_segment_capacity_mib, 410,
: "Capacity of protected segment of block cache in MiB");
: TAG_FLAG(block_cache_protected_segment_capacity_mib, experimental);
From the usability standpoint, maybe it's more better to introduce (total_capacity, segment_ratio) parameters pair instead? What do you think?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@93
PS9, Line 93: Cache* CreateCache(int64_t capacity) {
: const auto& mem_type = BlockCache::GetConfiguredCacheMemoryTypeOrDie();
: switch (mem_type) {
: case Cache::MemoryType::DRAM:
: return NewCache<Cache::EvictionPolicy::LRU, Cache::MemoryType::DRAM>(
: capacity, "block_cache");
: case Cache::MemoryType::NVM:
: return NewCache<Cache::EvictionPolicy::LRU, Cache::MemoryType::NVM>(
: capacity, "block_cache");
: default:
: LOG(FATAL) << "unsupported LRU cache memory type: " << mem_type;
: return nullptr;
: }
: }
Why not to turn this helper function into a static BlockCache::Create() function? In other words, why is it beneficial to have this as a stand-alone function?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@122
PS9, Line 122: std::
nit for here and below in this file: consider adding corresponding 'using' directive and dropping the 'std' prefix
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@123
PS9, Line 123: std::string temp;
: ToUpperCase(input, &temp);
: return temp;
Why to introduce this if there is already ToUpperCase() in src/kudu/util/string_case.h ?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/dense_node-itest.cc
File src/kudu/integration-tests/dense_node-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/dense_node-itest.cc@174
PS9, Line 174: FLAGS_block_cache_eviction_policy = "SLRU";
: opts.extra_master_flags.emplace_back(Substitute("--block_cache_eviction_policy=$0",
: FLAGS_block_cache_eviction_policy));
: opts.extra_tserver_flags.emplace_back(Substitute("--block_cache_eviction_policy=$0",
: FLAGS_block_cache_eviction_policy));
By doing this, we are removing existing test coverage for the LRU block cache.
Shouldn't corresponding test scenarios be parameterized based on the type of the block cache instead?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/full_stack-insert-scan-test.cc@147
PS9, Line 147: FLAGS_block_cache_eviction_policy = "SLRU";
Shouldn't this become a parameter of the corresponding test scenarios instead?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/heavy-update-compaction-itest.cc
File src/kudu/integration-tests/heavy-update-compaction-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/integration-tests/heavy-update-compaction-itest.cc@119
PS9, Line 119: FLAGS_block_cache_eviction_policy = "SLRU";
Shouldn't this become a parameter for the scenarios of this test suite instead?
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/tserver/tablet_server-stress-test.cc
File src/kudu/tserver/tablet_server-stress-test.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/tserver/tablet_server-stress-test.cc@80
PS9, Line 80: FLAGS_block_cache_eviction_policy = "SLRU";
Shouldn't this become a parameter for the scenarios of this test suite instead?
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 9
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Aug 2024 18:18:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#13).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Many integration tests that use the block cache
are also updated to include the block cache of
SLRU eviction policy.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
8 files changed, 351 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/13
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 13
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 6: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/340/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 20:42:51 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/21390/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21390/2//COMMIT_MSG@13
PS2, Line 13: The default
: value is still the old LRU cache.
I think it's crucial to enable at least a few integration-style scenarios using the block cache backed by the new SLRU implementation.
I'd think of scenarios in tablet_server_stress-test.cc, full_stack-insert-scan-test.cc, dense_node-itest.cc and heavy-update-compaction-itest.cc
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@46
PS2, Line 46: Either 'LRU' (default) or 'SLRU' (experimental
Please add a validator for the value, and make this flag case-insensitive, so "LRU" and "lru" both would work (as well as "lRu").
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@46
PS2, Line 46:
nit: drop the trailing space?
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@49
PS2, Line 49: block_cache_probationary_segment_capacity_mb
For this the block_cache_protected_segment_capacity_mb flag below: Does it make sense to add a validator for this flag to reject at least negative values? From the other side, why not to use unsigned type for this flag?
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@50
PS2, Line 50: MB
here and below: is this actually MiB/MiBytes?
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@58
PS2, Line 58: ,"
nit: reconsider the punctuation and the result message for the flag
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@171
PS2, Line 171: "Lower --block_cache_probationary_segment_capacity_mb and"
nit: add space after 'and'
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@194
PS2, Line 194: FLAGS_block_cache_eviction_policy
Please make sure the value of this flag is treated as case-insensitive. Doing that in just one place and then reusing the function that converts a string into a enumeration should be robust enough.
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@222
PS2, Line 222: ToUpperCase(FLAGS_block_cache_eviction_policy, &FLAGS_block_cache_eviction_policy);
I'm not sure it's a good idea to update the actual value of the flag like this: it might quite unexpected behavior from the user's perspective. Why not to use a temporary variable for converting to the upper case?
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 May 2024 17:56:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@235
PS2, Line 235: BlockCache::BlockCache()
: : BlockCache(FLAGS_block_cache_capacity_mb * 1024 * 1024) {
: }
Interesting, but why this constructor isn't updated to take care of different type of the underlying cache? That's especially strange given the funny code below for the next constructor when the 'capacity' parameter is ignored upon using the SLRU type of the cache.
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@242
PS2, Line 242: CreateCache(FLAGS_block_cache_probationary_segment_capacity_mb * 1024 * 1024,
: FLAGS_block_cache_protected_segment_capacity_mb * 1024 * 1024,
: FLAGS_block_cache_lookups_before_upgrade)) {
That's a bad smell -- how 'capacity' is then used in this branch?
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@247
PS2, Line 247: BlockCache::BlockCache(size_t probationary_segment_capacity,
: size_t protected_segment_capacity,
: size_t lookups)
: : cache_(CreateCache(probationary_segment_capacity, protected_segment_capacity, lookups)) {
: }
With this code and the code above it smells even worse -- now there is a complete mess with trying to understand when a particular type of cache is created based on the constructor invoked and the --block_cache_eviction_policy setting.
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 May 2024 18:29:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/105/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 05 May 2024 20:19:02 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 3:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/191/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 May 2024 17:06:50 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 11:
(9 comments)
overall looks OK, just a few questions and nits at this point
http://gerrit.cloudera.org:8080/#/c/21390/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21390/11//COMMIT_MSG@15
PS11, Line 15:
I think it would be great to outline the criteria you used to select tests updated in this patch with introducing SLRU cache policy in addition to existing LRU.
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@93
PS9, Line 93: class MetricEntity;
:
: namespace cfile {
:
: namespace {
:
: Cache* CreateCache(int64_t capacity) {
: const auto& mem_type = BlockCache::GetConfiguredCacheMemoryTypeOrDie();
: switch (mem_type) {
: case Cache::MemoryType::DRAM:
: return NewCache<Cache::EvictionPolicy::LRU, Cache::MemoryType::DRAM>(
: capacity, "block_cache");
: case Cache::MemoryType::NVM:
:
> I guess a private static function would accomplish the same thing. I don't
One benefit of moving this into BlockCache is the ability to contain GetConfiguredCacheMemoryTypeOrDie() and other guts of the internal implementation within the BlockCache itself, not exposing it outside.
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@123
PS9, Line 123: LOG(FATAL) << "unsupported SLRU cache memory type: " << mem_type;
: return nullptr;
: }
> It's used several times in this file. IMO it's cleaner to make this functio
Ah, indeed -- that's just a wrapper -- for some reason I assumed it's something more than a wrapper. Should be OK then.
Maybe, this means that the signature of ToUpperCase() should have been changed. I'm not sure why it uses the out parameter to return the result, not just returning it. Anyway, that's a separate question.
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/block_cache.cc@182
PS11, Line 182: ToUpper(FLAGS_block_cache_type) != "DRAM" ||
Why "DRAM" is so important here? Please add a comment since it's not very obvious for a reader without much context..
Also, how to not forgetting to updated this once adding NVMe-based implementation for the SLRU cache?
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/block_cache.cc@251
PS11, Line 251: int64_t probationary_capacity =
nit for here and elsewhere with a conditional on Cache::EvictionPolicy: add DCHECK() for the expected type of the cache here or rewrite this using switch() with corresponding action for 'default' to make it easier to detect a need of updating this code if a new eviction is added later on?
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc
File src/kudu/cfile/cfile-test.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc@425
PS11, Line 425: lru
nit: maybe, use uppercase here for consistency with other flags in the code around?
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc@428
PS11, Line 428: slru
ditto
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc@439
PS11, Line 439: lru
ditto
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/integration-tests/heavy-update-compaction-itest.cc
File src/kudu/integration-tests/heavy-update-compaction-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/integration-tests/heavy-update-compaction-itest.cc@196
PS11, Line 196: Singleton<cfile::BlockCache>::UnsafeReset();
Would it be more consistent to call this in TearDown(), just in case any extra scenarios added later on?
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 11
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Aug 2024 01:53:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 14: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/integration-tests/heavy-update-compaction-itest.cc
File src/kudu/integration-tests/heavy-update-compaction-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/integration-tests/heavy-update-compaction-itest.cc@196
PS11, Line 196: Singleton<cfile::BlockCache>::UnsafeReset();
> In theory yes, but I already tried adding this to TearDown() but there were
OK, I guess that need to be clarified later on when somebody adds an extra test scenario :)
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 14
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Aug 2024 06:02:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 9: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/362/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 9
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Jul 2024 09:40:02 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 2:
Build Aborted
http://jenkins.kudu.apache.org/job/pre_commit/105/ : ABORTED
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 06 May 2024 11:17:19 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 8: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/357/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 8
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jul 2024 09:54:38 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 14:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/432/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 14
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 22:13:31 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#2).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
2 files changed, 139 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/2
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 10:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/421/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 10
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 23:17:26 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 11:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/422/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 11
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 23:21:31 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 7:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/355/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 7
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jul 2024 06:55:42 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 12: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/427/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 12
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 17 Aug 2024 02:30:42 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 5: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/243/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Jun 2024 21:26:59 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/88/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 May 2024 20:47:38 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 2:
Build Aborted
http://jenkins.kudu.apache.org/job/pre_commit/100/ : ABORTED
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 05 May 2024 20:18:36 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#7).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
6 files changed, 194 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/7
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 7
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 4:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/196/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2024 20:54:38 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#12).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Many integration tests that use the block cache
are also updated to include the block cache of
SLRU eviction policy.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
8 files changed, 347 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/12
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 12
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#14).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Many integration tests that use the block cache
are also updated to include the block cache of
SLRU eviction policy.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
8 files changed, 351 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/14
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 14
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 3:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/21390/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21390/2//COMMIT_MSG@13
PS2, Line 13: The default
: value is still the old LRU cache.
> I think it's crucial to enable at least a few integration-style scenarios u
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@46
PS2, Line 46: .
> nit: drop the trailing space?
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@46
PS2, Line 46: Either 'LRU' (default) or 'SLRU' (experimental
> Please add a validator for the value, and make this flag case-insensitive,
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@49
PS2, Line 49: block_cache_probationary_segment_capacity_mi
> For this the block_cache_protected_segment_capacity_mb flag below: Does it
Added a check in the validators to reject negative values. In the validators, the flag is compared to the memory pressure threshold which is an int64_t so it makes more sense to keep the flag as int64.
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@50
PS2, Line 50: Mi
> here and below: is this actually MiB/MiBytes?
Yep since it's on the binary scale. Should I change it for block_cache_capacity_mb too?
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@58
PS2, Line 58: .
> nit: reconsider the punctuation and the result message for the flag
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@171
PS2, Line 171: LOG(ERROR) << Substitute("Probationary segment capacity of block cache must be > 0. "
> nit: add space after 'and'
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@194
PS2, Line 194: capacity > mpt / 2) {
> Please make sure the value of this flag is treated as case-insensitive. Do
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@222
PS2, Line 222:
> I'm not sure it's a good idea to update the actual value of the flag like t
Done
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@235
PS2, Line 235: return Cache::MemoryType::DRAM;
: }
:
> Interesting, but why this constructor isn't updated to take care of differe
Made the constructor more readable now.
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@242
PS2, Line 242:
: Cache::EvictionPolicy BlockCache::GetCacheEvictionPolicyOrDie() {
: const auto& eviction_policy = ToUpper(FLAGS_block_cache_eviction_po
> That's a bad smell -- how 'capacity' is then used in this branch?
Moved this code to private constructor.
http://gerrit.cloudera.org:8080/#/c/21390/2/src/kudu/cfile/block_cache.cc@247
PS2, Line 247: }
: if (eviction_policy == "SLRU") {
: return Cache::EvictionPolicy::SLRU;
: }
:
> With this code and the code above it smells even worse -- now there is a co
Done
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 May 2024 17:07:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#3).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
6 files changed, 191 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/3
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/191/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 May 2024 17:19:26 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#4).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
6 files changed, 192 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/4
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 12:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/427/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 12
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 17 Aug 2024 00:30:51 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 7: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/355/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 7
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jul 2024 07:01:21 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Many integration tests that use the block cache
are also updated to include the block cache of
SLRU eviction policy.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Reviewed-on: http://gerrit.cloudera.org:8080/21390
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
8 files changed, 351 insertions(+), 88 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 15
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 14
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 8:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/357/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 8
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jul 2024 07:08:26 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/196/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2024 21:07:50 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 5:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/243/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Jun 2024 20:05:47 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/100/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 03 May 2024 18:12:27 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 9:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/362/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 9
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Jul 2024 07:56:01 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 6:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/340/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 20:12:13 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 14: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/432/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 14
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 23:01:44 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#11).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
8 files changed, 319 insertions(+), 74 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/11
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 11
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 13:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/431/
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 13
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 21:18:11 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 14: Verified+1
unrelated test failures
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 14
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Aug 2024 06:02:27 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 12:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/21390/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21390/11//COMMIT_MSG@15
PS11, Line 15:
> I think it would be great to outline the criteria you used to select tests
Done
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@93
PS9, Line 93: class MetricEntity;
:
: namespace cfile {
:
: namespace {
:
: string ToUpper(const string& input) {
: string temp;
: ToUpperCase(input, &temp);
: return temp;
: }
:
: } // anonymous namespace
:
> One benefit of moving this into BlockCache is the ability to contain GetCon
Done
http://gerrit.cloudera.org:8080/#/c/21390/9/src/kudu/cfile/block_cache.cc@123
PS9, Line 123: "--memory_limit_hard_bytes.",
: capacity, mpt);
: return fal
> Ah, indeed -- that's just a wrapper -- for some reason I assumed it's somet
Done
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/block_cache.cc@182
PS11, Line 182: bool ValidateEvictionPolicy() {
> Why "DRAM" is so important here? Please add a comment since it's not very
The capacity validators aren't necessary for NVM-based caches according to KUDU-2920 so adding that won't be a problem. Added a comment in the header file where both the capacity validator functions are defined.
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/block_cache.cc@251
PS11, Line 251: DCHECK(eviction_policy == Cache::EvictionPolicy::SLRU);
> nit for here and elsewhere with a conditional on Cache::EvictionPolicy: add
Done
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc
File src/kudu/cfile/cfile-test.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc@425
PS11, Line 425: LRU
> nit: maybe, use uppercase here for consistency with other flags in the code
Done
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc@428
PS11, Line 428: SLRU
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/cfile/cfile-test.cc@439
PS11, Line 439: LRU
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/integration-tests/heavy-update-compaction-itest.cc
File src/kudu/integration-tests/heavy-update-compaction-itest.cc:
http://gerrit.cloudera.org:8080/#/c/21390/11/src/kudu/integration-tests/heavy-update-compaction-itest.cc@196
PS11, Line 196: Singleton<cfile::BlockCache>::UnsafeReset();
> Would it be more consistent to call this in TearDown(), just in case any ex
In theory yes, but I already tried adding this to TearDown() but there were errors when resetting the block cache, so this was the only way that worked.
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 12
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 17 Aug 2024 00:32:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21390
to look at the new patch set (#10).
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
KUDU-613: Integrate SLRU cache into block cache
This patch integrates the new SLRU cache into the
block cache. It adds several flags to make the SLRU
cache configurable for capacity and lookups as well
as adding an eviction_policy flag to give users
the option to use the new SLRU cache. The default
value is still the old LRU cache.
Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
---
M src/kudu/cfile/block_cache-test.cc
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/integration-tests/dense_node-itest.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/integration-tests/heavy-update-compaction-itest.cc
M src/kudu/tserver/tablet_server-stress-test.cc
8 files changed, 320 insertions(+), 74 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/21390/10
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 10
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-613: Integrate SLRU cache into block cache
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21390 )
Change subject: KUDU-613: Integrate SLRU cache into block cache
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/88/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/21390
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04411ab2756045f15a272f3397d46d871b087b03
Gerrit-Change-Number: 21390
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 May 2024 21:48:57 +0000
Gerrit-HasComments: No