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:18 UTC

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19930


Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................

IMPALA-10971: Fix data cache metrics for instant evictions

When inserting an entry into the data cache, the
Insert() can fail and instantly evict the new entry.
This is currently only true for the LIRS cache
eviction policy. When this happens, the cache metrics
are not maintained properly. The instant eviction
results in an unmatched decrement to the total bytes
and num entries counters.

This moves the increments to happen prior to the
Insert() call, which fixes the instant eviction
case. The behavior does not change noticeably for
the successful Insert() case.

Testing:
 - Added custom cluster test to verify this case

Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
---
M be/src/runtime/io/data-cache.cc
M tests/custom_cluster/test_data_cache.py
2 files changed, 47 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/19930/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 2: Code-Review+2

Carry +2


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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: Wed, 24 May 2023 18:50:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13114/ : 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/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 18:52:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 18:36:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9366/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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:03:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9346/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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: Wed, 24 May 2023 18:55:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 4: Code-Review+2

Carry +2


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 3: Code-Review+2

Carry +2


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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 17:48:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 4: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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:20:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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 00:13:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10971: Fix data cache metrics for instant evictions

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19930 )

Change subject: IMPALA-10971: Fix data cache metrics for instant evictions
......................................................................

IMPALA-10971: Fix data cache metrics for instant evictions

When inserting an entry into the data cache, the
Insert() can fail and instantly evict the new entry.
This is currently only true for the LIRS cache
eviction policy. When this happens, the cache metrics
are not maintained properly. The instant eviction
results in an unmatched decrement to the total bytes
and num entries counters.

This moves the increments to happen prior to the
Insert() call, which fixes the instant eviction
case. The behavior does not change noticeably for
the successful Insert() case.

Testing:
 - Added custom cluster test to verify this case

Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Reviewed-on: http://gerrit.cloudera.org:8080/19930
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/io/data-cache.cc
M tests/custom_cluster/test_data_cache.py
2 files changed, 47 insertions(+), 6 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5db3eaca2f4459844e3270846d905f76265b6b3e
Gerrit-Change-Number: 19930
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>