[kudu-CR] KUDU-613: Cleanup of cache code
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. This path also templatizes the HandleTable class to be used by both the cache and nvm_cache files. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Reviewed-on: http://gerrit.cloudera.org:8080/21018 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/file_cache.cc M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h 5 files changed, 206 insertions(+), 287 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 10 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-613: Cleanup of cache code
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 9 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 May 2024 16:57:50 + Gerrit-HasComments: No
[kudu-CR] KUDU-613: Cleanup of cache code
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. Patch Set 9: Verified+1 Build Successful http://jenkins.kudu.apache.org/job/pre_commit/98/ : SUCCESS -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 9 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 May 2024 22:26:22 + Gerrit-HasComments: No
[kudu-CR] KUDU-613: Cleanup of cache code
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. Patch Set 9: Build Started http://jenkins.kudu.apache.org/job/pre_commit/98/ -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 9 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 May 2024 18:12:27 + Gerrit-HasComments: No
[kudu-CR] KUDU-613: Cleanup of cache code
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. Patch Set 8: Verified-1 Build Failed http://jenkins.kudu.apache.org/job/pre_commit/87/ : FAILURE -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 May 2024 23:05:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-613: Cleanup of cache code
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/21018/7/src/kudu/util/nvm_cache.cc File src/kudu/util/nvm_cache.cc: http://gerrit.cloudera.org:8080/#/c/21018/7/src/kudu/util/nvm_cache.cc@a223 PS7, Line 223: : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : > If it doesn't seem feasible to converge on LRUHandle, is it possible to con Done -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 May 2024 20:48:58 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21018 to look at the new patch set (#8). Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. This path also templatizes the HandleTable class to be used by both the cache and nvm_cache files. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf --- M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/file_cache.cc M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h 5 files changed, 206 insertions(+), 287 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/21018/8 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-613: Cleanup of cache code
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21018 ) Change subject: KUDU-613: Cleanup of cache code .. Patch Set 8: Build Started http://jenkins.kudu.apache.org/job/pre_commit/87/ -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 May 2024 20:47:38 + Gerrit-HasComments: No
[kudu-CR] KUDU-613: Cleanup of cache code
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/21018 )
Change subject: KUDU-613: Cleanup of cache code
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21018/7/src/kudu/util/nvm_cache.cc
File src/kudu/util/nvm_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21018/7/src/kudu/util/nvm_cache.cc@a223
PS7, Line 223:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
:
If it doesn't seem feasible to converge on LRUHandle, is it possible to
converge at least on one (maybe templatized?) implementation of HandleTable
class to be re-used by the code in nvm_cache.{cc, h} and cache.{cc, h}?
--
To view, visit http://gerrit.cloudera.org:8080/21018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf
Gerrit-Change-Number: 21018
Gerrit-PatchSet: 7
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 09 Apr 2024 18:36:24 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Mahesh Reddy has posted comments on this change. (
http://gerrit.cloudera.org:8080/21018 )
Change subject: KUDU-613: Cleanup of cache code
..
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/cache.h
File src/kudu/util/cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/cache.h@70
PS6, Line 70: struct RLHandle {
> So, the decision is not to try unifying this and util::LRUHandle from nvm_c
The key and value data are represented differently between this and LRUHandle
from nvm_cache.h. I decided to leave it as is for now.
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h
File src/kudu/util/nvm_cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h@34
PS2, Line 34: struct LRUHandle {
> It might be something related to the use cases, but overall that's quite su
Done
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/nvm_cache.cc
File src/kudu/util/nvm_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/nvm_cache.cc@200
PS6, Line 200: return *FindP
> nit for here and elsewhere in this file: consider introducing corresponding
Done
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/nvm_cache.cc@397
PS6, Line 397:
> nit: wrong indent?
Done
--
To view, visit http://gerrit.cloudera.org:8080/21018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf
Gerrit-Change-Number: 21018
Gerrit-PatchSet: 7
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 01 Mar 2024 00:23:13 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21018 to look at the new patch set (#7). Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf --- M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/file_cache.cc M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h 5 files changed, 207 insertions(+), 203 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/21018/7 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 7 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-613: Cleanup of cache code
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/21018 )
Change subject: KUDU-613: Cleanup of cache code
..
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/cache.h
File src/kudu/util/cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/cache.h@70
PS6, Line 70: struct RLHandle {
So, the decision is not to try unifying this and util::LRUHandle from
nvm_cache.h?
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/nvm_cache.cc
File src/kudu/util/nvm_cache.cc:
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/nvm_cache.cc@200
PS6, Line 200: util::LRUHandle
nit for here and elsewhere in this file: consider introducing corresponding
'using' directive instead and dropping the 'util::' namespace prefix
http://gerrit.cloudera.org:8080/#/c/21018/6/src/kudu/util/nvm_cache.cc@397
PS6, Line 397:
nit: wrong indent?
--
To view, visit http://gerrit.cloudera.org:8080/21018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf
Gerrit-Change-Number: 21018
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 29 Feb 2024 02:33:21 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21018 to look at the new patch set (#6). Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf --- M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/file_cache.cc M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h 5 files changed, 268 insertions(+), 266 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/21018/6 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 6 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-613: Cleanup of cache code
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/21018 )
Change subject: KUDU-613: Cleanup of cache code
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h
File src/kudu/util/nvm_cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h@34
PS2, Line 34: struct LRUHandle {
> I'll look into it, there's some ASAN errors from trying to templatize the H
It might be something related to the use cases, but overall that's quite
surprising to hear about ASAN errors on that path.
Another point is: while you are at this, maybe consider replacing Atomic32 with
std::atomic at least?
--
To view, visit http://gerrit.cloudera.org:8080/21018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf
Gerrit-Change-Number: 21018
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 13 Feb 2024 20:29:02 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Mahesh Reddy has posted comments on this change. (
http://gerrit.cloudera.org:8080/21018 )
Change subject: KUDU-613: Cleanup of cache code
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/cache.h
File src/kudu/util/cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/cache.h@81
PS2, Line 81: // The storage for the key/value pair itself. The data is
stored as:
: // [key bytes ...] [padding up to 8-byte boundary] [value
bytes ...]
: uint8_t kv_data[1
> Is it possible to introduce this field only in the patch implementing the S
Done
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h
File src/kudu/util/nvm_cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h@34
PS2, Line 34: struct LRUHandle {
> Any chance to unify Cache::RLHandle and this handle by templatizing? Align
I'll look into it, there's some ASAN errors from trying to templatize the
HandleDeleter and PendingHandleDeleter classes that I think are related to the
file_cache somehow, still need to debug that first.
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h@46
PS2, Line 46: Slice key() const {
: return Slice(kv_data, key_length);
: }
> Ditto: is it possible to introduce this field only in the patch implementin
Done
--
To view, visit http://gerrit.cloudera.org:8080/21018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf
Gerrit-Change-Number: 21018
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Feb 2024 22:03:47 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21018 to look at the new patch set (#3). Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. This patch also templatizes the HandleDeleter and the PendingHandleDeleter class as it will also be used by the SLRU cache. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf --- M src/kudu/cfile/block_cache.h M src/kudu/master/table_locations_cache.cc M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/file_cache.cc M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h M src/kudu/util/ttl_cache.h 8 files changed, 290 insertions(+), 286 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/21018/3 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 3 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-613: Cleanup of cache code
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/21018 )
Change subject: KUDU-613: Cleanup of cache code
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/cache.h
File src/kudu/util/cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/cache.h@81
PS2, Line 81: // Number of lookups for this entry. Used for upgrading to
protected segment in SLRU cache.
: // Resets to 0 when moved between probationary and protected
segments in both directions.
: uint32_t lookups;
Is it possible to introduce this field only in the patch implementing the SLRU
cache?
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h
File src/kudu/util/nvm_cache.h:
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h@34
PS2, Line 34: struct LRUHandle {
Any chance to unify Cache::RLHandle and this handle by templatizing?
Alignment-related functionality might be introduced as an option (a template
parameter).
http://gerrit.cloudera.org:8080/#/c/21018/2/src/kudu/util/nvm_cache.h@46
PS2, Line 46: // Number of lookups for this entry. Used for upgrading to
protected segment in SLRU cache.
: // Resets to 0 when moved between probationary and protected
segments in both directions.
: uint32_t lookups;
Ditto: is it possible to introduce this field only in the patch implementing
the SLRU cache?
--
To view, visit http://gerrit.cloudera.org:8080/21018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf
Gerrit-Change-Number: 21018
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Feb 2024 02:31:31 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-613: Cleanup of cache code
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21018 to look at the new patch set (#2). Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. This patch also templatizes the HandleDeleter and the PendingHandleDeleter class as it will also be used by the SLRU cache. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf --- M src/kudu/cfile/block_cache.h M src/kudu/master/table_locations_cache.cc M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/file_cache.cc M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h M src/kudu/util/ttl_cache.h 8 files changed, 297 insertions(+), 286 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/21018/2 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 2 Gerrit-Owner: Mahesh Reddy Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-613: Cleanup of cache code
Mahesh Reddy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21018 Change subject: KUDU-613: Cleanup of cache code .. KUDU-613: Cleanup of cache code This patch moves some classes out of the anonymous namespace and into the headers of the cache and nvm_cache files. These classes will be used by the new SLRU cache. This patch also templatizes the HandleDeleter and the PendingHandleDeleter class as it will also be used by the SLRU cache. Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf --- M src/kudu/cfile/block_cache.h M src/kudu/master/table_locations_cache.cc M src/kudu/util/cache.cc M src/kudu/util/cache.h M src/kudu/util/nvm_cache.cc M src/kudu/util/nvm_cache.h M src/kudu/util/ttl_cache.h 7 files changed, 296 insertions(+), 285 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/21018/1 -- To view, visit http://gerrit.cloudera.org:8080/21018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I506d4577c0ae873b01d7fa4f53846d6fd0f664cf Gerrit-Change-Number: 21018 Gerrit-PatchSet: 1 Gerrit-Owner: Mahesh Reddy
