You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2023/05/24 18:29:15 UTC
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19929
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
The LIRS implementation relies on the invariant that the
last entry on the recency list is PROTECTED. This is not
being enforced properly when the code removes an entry
from the recency list in ToUninitialized(). This comes
up when an entry is erased or when a protected element
is overwritten.
This modifies ToUninitialized() to call TrimRecencyList()
when removing the last entry on the recency list. To
avoid recursion, it avoids this logic when TrimRecencyList()
itself is calling ToUninitialized().
Tests:
- Added a unit tests for erasing/overwriting the last entry
on the recency list
Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
---
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
2 files changed, 118 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19929/1
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
Patch Set 4: Code-Review+1
Carry +1
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 17:02:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 22:23:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19929/1/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/19929/1/be/src/util/cache/lirs-cache.cc@793
PS1, Line 793: void LIRSCacheShard::ToUninitialized(LIRSThreadState* tstate, LIRSHandle* e, bool is_trim) {
> line too long (92 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 18:50:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13115/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 19:13:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19929
to look at the new patch set (#4).
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
The LIRS implementation relies on the invariant that the
last entry on the recency list is PROTECTED. This is not
being enforced properly when the code removes an entry
from the recency list in ToUninitialized(). This comes
up when an entry is erased or when a protected element
is overwritten.
This modifies ToUninitialized() to call TrimRecencyList()
when removing the last entry on the recency list. To
avoid recursion, it avoids this logic when TrimRecencyList()
itself is calling ToUninitialized().
Tests:
- Added a unit tests for erasing/overwriting the last entry
on the recency list
Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
---
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
2 files changed, 123 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19929/4
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 18:49:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13113/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 18:52:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache-test.cc
File be/src/util/cache/lirs-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache-test.cc@288
PS2, Line 288: // Replace an protected key with a new value
nit: an -> a
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache.cc@696
PS2, Line 696: DCHECK(&*recency_list_.begin() == e);
nit: I think this could be &recency_list_.front().
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 16:17:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19929
to look at the new patch set (#2).
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
The LIRS implementation relies on the invariant that the
last entry on the recency list is PROTECTED. This is not
being enforced properly when the code removes an entry
from the recency list in ToUninitialized(). This comes
up when an entry is erased or when a protected element
is overwritten.
This modifies ToUninitialized() to call TrimRecencyList()
when removing the last entry on the recency list. To
avoid recursion, it avoids this logic when TrimRecencyList()
itself is calling ToUninitialized().
Tests:
- Added a unit tests for erasing/overwriting the last entry
on the recency list
Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
---
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
2 files changed, 119 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19929/2
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
Patch Set 4:
Fixed grammar in title.
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 17:02:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache-test.cc
File be/src/util/cache/lirs-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache-test.cc@288
PS2, Line 288: // Replace an protected key with a new value
> nit: an -> a
Done
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/19929/2/be/src/util/cache/lirs-cache.cc@696
PS2, Line 696: DCHECK(&*recency_list_.begin() == e);
> nit: I think this could be &recency_list_.front().
That is cleaner, so I switched the other locations as well.
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 17:47:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13130/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 18:09:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19929/1/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/19929/1/be/src/util/cache/lirs-cache.cc@793
PS1, Line 793: void LIRSCacheShard::ToUninitialized(LIRSThreadState* tstate, LIRSHandle* e, bool is_trim) {
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 18:30:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19929
to look at the new patch set (#3).
Change subject: IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
......................................................................
IMPALA-12154: Trim recency list appropriate for LIRS ToUninitialize()
The LIRS implementation relies on the invariant that the
last entry on the recency list is PROTECTED. This is not
being enforced properly when the code removes an entry
from the recency list in ToUninitialized(). This comes
up when an entry is erased or when a protected element
is overwritten.
This modifies ToUninitialized() to call TrimRecencyList()
when removing the last entry on the recency list. To
avoid recursion, it avoids this logic when TrimRecencyList()
itself is calling ToUninitialized().
Tests:
- Added a unit tests for erasing/overwriting the last entry
on the recency list
Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
---
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
2 files changed, 123 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19929/3
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13185/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 17:24:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Jun 2023 18:31:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19929 )
Change subject: IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
......................................................................
IMPALA-12154: Trim recency list appropriately for LIRS ToUninitialize()
The LIRS implementation relies on the invariant that the
last entry on the recency list is PROTECTED. This is not
being enforced properly when the code removes an entry
from the recency list in ToUninitialized(). This comes
up when an entry is erased or when a protected element
is overwritten.
This modifies ToUninitialized() to call TrimRecencyList()
when removing the last entry on the recency list. To
avoid recursion, it avoids this logic when TrimRecencyList()
itself is calling ToUninitialized().
Tests:
- Added a unit tests for erasing/overwriting the last entry
on the recency list
Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Reviewed-on: http://gerrit.cloudera.org:8080/19929
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
2 files changed, 123 insertions(+), 8 deletions(-)
Approvals:
Joe McDonnell: Looks good to me, but someone else must approve
Michael Smith: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19929
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I83298e0a042174a09f0144aa336b2eb2b28bfee8
Gerrit-Change-Number: 19929
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>