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 2024/03/08 05:35:57 UTC
[Impala-ASF-CR] IMPALA-12883: Support updating the charge an entry in the cache
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21122
Change subject: IMPALA-12883: Support updating the charge an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to modify its behavior as it
is approaching the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 325 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/1
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Reviewed-on: http://gerrit.cloudera.org:8080/21122
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Yida Wu <wy...@gmail.com>
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 514 insertions(+), 91 deletions(-)
Approvals:
Michael Smith: Looks good to me, but someone else must approve
Impala Public Jenkins: Verified
Yida Wu: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 8
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Mar 2024 20:53:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc
File be/src/util/cache/cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc@180
PS3, Line 180: // Updating the charge for the cache entry evicts something
> Oops, fixed
Ack
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 18:38:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10369/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 09 Mar 2024 00:06:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc
File be/src/util/cache/cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc@180
PS3, Line 180: // Updating the charge for the erased element evicts something
Why do you call h1 the erased element?
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h
File be/src/util/cache/cache.h:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@96
PS2, Line 96: // this makes it easier to embed in containers.
> Done
Ack
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@130
PS2, Line 130: // this makes it easier to embed in containers.
> Done
Ack
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/lirs-cache-test.cc
File be/src/util/cache/lirs-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/lirs-cache-test.cc@290
PS3, Line 290: cache_->UpdateCharge(handle, cache_->MaxCharge());
Do you want to validate the handle's charge doesn't change too?
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc@975
PS2, Line 975: handle->set_charge(charge);
> Fixed this and added some tests
Done
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Mar 2024 18:53:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15461/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 06:02:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/4/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/4/be/src/util/cache/lirs-cache.cc@865
PS4, Line 865: HandleBase* LIRSCacheShard::Allocate(Slice key, uint32_t hash, int val_len, size_t charge) {
> line too long (92 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 17:47:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10373/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 18:57:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h
File be/src/util/cache/cache.h:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@96
PS2, Line 96: // this makes it easier to embed this containers.
> nit: embed in containers
Done
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@130
PS2, Line 130: // this makes it easier to embed this containers.
> nit: embed in containers
Done
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 23:42:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10369/
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 09 Mar 2024 04:54:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 5: Code-Review+1
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc
File be/src/util/cache/lirs-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc@274
PS5, Line 274:
nit. unnecessary new line
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc@424
PS5, Line 424:
nit. unnecessary new line
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc@455
PS5, Line 455:
nit. unnecessary new line
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache.cc@990
PS5, Line 990: CHECK(false);
Maybe add a logging for unexpected state?
CHECK(false) << "Unexpected state: " << e->state();
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/rl-cache.cc
File be/src/util/cache/rl-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/rl-cache.cc@324
PS5, Line 324: while (usage_ > capacity_ && rl_.next != &rl_) {
: RLHandle* old = rl_.next;
: RL_Remove(old);
: table_.Remove(old->key(), old->hash());
: if (Unref(old)) {
: old->next = to_remove_head;
: to_remove_head = old;
: }
: }
This section of code is identical to a part of RLCacheShard<policy>::Insert. Could we consider extracting this segment into a separate function for it to reduce redundancy?
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/rl-cache.cc@336
PS5, Line 336: while (to_remove_head != nullptr) {
: RLHandle* next = to_remove_head->next;
: FreeEntry(to_remove_head);
: to_remove_head = next;
: }
Same as above
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 14 Mar 2024 18:03:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc@975
PS2, Line 975: // The handle was marked for eviction while resident; will be evicted on release.
> Should we avoid calling UpdateMemTracker for this case? I think we do becau
Now that I think about it, I think the update of the charge should happen under a lock. We don't know if something else could evict this entry and whether it saw the old value or new value.
Maybe the logic should be:
1. Get lock
2. Bail out without updating the charge if this is TOMBSTONE or UNINITIALIZED. The charge shouldn't change for those entries. They aren't counted towards the capacity.
3. Proceed with the PROTECTED/UNPROTECTED cases.
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 19:03:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc@975
PS2, Line 975: // The handle was marked for eviction while resident; will be evicted on release.
> That seems reasonable.
Fixed this and added some tests
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 23:43:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/6/be/src/util/cache/rl-cache.cc
File be/src/util/cache/rl-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/6/be/src/util/cache/rl-cache.cc@410
PS6, Line 410: template<Cache::EvictionPolicy policy>
nit: seems a little out of place to declare it here. Why not right after EnforceCapacity?
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 23:52:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 2: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h
File be/src/util/cache/cache.h:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@96
PS2, Line 96: // this makes it easier to embed this containers.
nit: embed in containers
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@130
PS2, Line 130: // this makes it easier to embed this containers.
nit: embed in containers
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc@975
PS2, Line 975: // The handle was marked for eviction while resident; will be evicted on release.
Should we avoid calling UpdateMemTracker for this case? I think we do because the handle references the new charge, and we need to add the delta so that eviction can subtract the new charge correctly. Might be worth adding a comment to clarify that.
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 18:01:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/6/be/src/util/cache/rl-cache.cc
File be/src/util/cache/rl-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/6/be/src/util/cache/rl-cache.cc@410
PS6, Line 410: template<Cache::EvictionPolicy policy>
> nit: seems a little out of place to declare it here. Why not right after En
Good point, I will move it.
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 23:53:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21122
to look at the new patch set (#6).
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 514 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/6
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 23:35:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21122
to look at the new patch set (#2).
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 325 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/2
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15471/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 09 Mar 2024 00:05:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21122
to look at the new patch set (#4).
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 438 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/4
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21122
to look at the new patch set (#3).
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 415 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/3
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Mar 2024 10:34:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10370/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Mar 2024 16:17:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 7: Code-Review+2
Carry Michael's +1.
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 19 Mar 2024 17:16:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21122
to look at the new patch set (#5).
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 439 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/5
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15486/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 18:08:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10379/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Mar 2024 05:45:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15544/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Mar 2024 00:09:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc@975
PS2, Line 975: // The handle was marked for eviction while resident; will be evicted on release.
> Now that I think about it, I think the update of the charge should happen u
That seems reasonable.
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 19:04:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Mar 2024 02:55:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 5:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc
File be/src/util/cache/lirs-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc@274
PS5, Line 274:
> nit. unnecessary new line
The comments in this file were somewhat like Python function comments (i.e. inside the function as the first few lines). Changed them to live above the TEST_F declaration, which is more C++ style. Dropped the extra space as part of that.
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc@424
PS5, Line 424:
> nit. unnecessary new line
Done
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache-test.cc@455
PS5, Line 455:
> nit. unnecessary new line
Done
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/lirs-cache.cc@990
PS5, Line 990: CHECK(false);
> Maybe add a logging for unexpected state?
Done
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/rl-cache.cc
File be/src/util/cache/rl-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/rl-cache.cc@324
PS5, Line 324: while (usage_ > capacity_ && rl_.next != &rl_) {
: RLHandle* old = rl_.next;
: RL_Remove(old);
: table_.Remove(old->key(), old->hash());
: if (Unref(old)) {
: old->next = to_remove_head;
: to_remove_head = old;
: }
: }
> This section of code is identical to a part of RLCacheShard<policy>::Insert
Refactored this to do something similar to what LIRS does with its LIRSThreadState. That let's this use a shared EnforceCapacity function and a shared CleanupThreadState function.
http://gerrit.cloudera.org:8080/#/c/21122/5/be/src/util/cache/rl-cache.cc@336
PS5, Line 336: while (to_remove_head != nullptr) {
: RLHandle* next = to_remove_head->next;
: FreeEntry(to_remove_head);
: to_remove_head = next;
: }
> Same as above
Done
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 23:45:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21122
to look at the new patch set (#7).
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
IMPALA-12883: Support updating the charge on an entry in the cache
The cache implementation currently assumes that a cache
entry's charge will remain constant over time and does
not support updating the charge. This works well for
existing cache users like the data cache and codegen
cache. However, it doesn't work as well for use cases
where the final size is not known up front. Being able
to update the charge also allows for avoiding
concurrency issues where two different threads are
trying to insert the same entry.
This adds the ability to update an entry's charge after
it has been inserted into the cache. This can trigger
evictions if the size increases. This also adds a way
to retrieve the maximum charge supported by the cache.
This allows a cache user to tune its behavior to avoid
generating entries that would exceed the maximum charge.
Testing:
- Added tests cases to the caching backend tests
Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
---
M be/src/util/cache/cache-internal.h
M be/src/util/cache/cache-test.cc
M be/src/util/cache/cache-test.h
M be/src/util/cache/cache.h
M be/src/util/cache/lirs-cache-test.cc
M be/src/util/cache/lirs-cache.cc
M be/src/util/cache/rl-cache-test.cc
M be/src/util/cache/rl-cache.cc
8 files changed, 514 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/21122/7
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15546/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Mar 2024 00:21:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15460/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Mar 2024 05:58:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3:
(2 comments)
I did one other change in the new upload: charge becomes a size_t in all interfaces (which means kAutomaticCharge is a size_t and set to the max val). That avoids issues about charge being negative, etc.
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc
File be/src/util/cache/cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc@180
PS3, Line 180: // Updating the charge for the erased element evicts something
> Why do you call h1 the erased element?
Oops, fixed
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/lirs-cache-test.cc
File be/src/util/cache/lirs-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/lirs-cache-test.cc@290
PS3, Line 290: cache_->UpdateCharge(handle, cache_->MaxCharge());
> Do you want to validate the handle's charge doesn't change too?
Added the ability to get the charge for a handle and checked that it doesn't change.
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 17:44:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21122/4/be/src/util/cache/lirs-cache.cc
File be/src/util/cache/lirs-cache.cc:
http://gerrit.cloudera.org:8080/#/c/21122/4/be/src/util/cache/lirs-cache.cc@865
PS4, Line 865: HandleBase* LIRSCacheShard::Allocate(Slice key, uint32_t hash, int val_len, size_t charge) {
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 17:45:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/15487/ : 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/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 18:12:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 )
Change subject: IMPALA-12883: Support updating the charge on an entry in the cache
......................................................................
Patch Set 3:
Test failure is not related
--
To view, visit http://gerrit.cloudera.org:8080/21122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025
Gerrit-Change-Number: 21122
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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Mar 2024 16:17:43 +0000
Gerrit-HasComments: No