Skip to content

Conversation

@avijayanhwx
Copy link
Contributor

@avijayanhwx avijayanhwx commented Oct 29, 2019

What changes were proposed in this pull request?

Added metrics for tracking the RocksDB keyMayExist miss rate. Fixed an issue in the Table.isExist() API such that we look at the out bound parameter that we pass in to rockdb.keyMayExist() before making a rockdb.get() call. The keyMayExist API usage was added in apache/hadoop#1013.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2364

How was this patch tested?

Unit tested.
Tested manually on cluster by looking at OzoneManager JMX endpoint.

@avijayanhwx avijayanhwx changed the title HDDS-2364. Add a OM metrics to find the false positive rate for the keyMayExist. HDDS-2364. Add OM metrics to find the false positive rate for the keyMayExist. Oct 29, 2019
@avijayanhwx
Copy link
Contributor Author

In this PR, my next commit will fix an issue in the isExist implementation, as discussed in #102 (review).

@avijayanhwx
Copy link
Contributor Author

cc @anuengineer

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM.
I will wait for @anuengineer review, as @anuengineer has asked for few changes in keyMayExist logic.

Copy link
Contributor

@anuengineer anuengineer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. LGTM.

@anuengineer anuengineer merged commit c83d5bc into apache:master Nov 13, 2019
StringBuilder outValue = new StringBuilder();
boolean keyMayExist = db.keyMayExist(handle, key, outValue);
if (keyMayExist) {
boolean keyExists = (outValue.length() > 0) ||
Copy link
Member

@maobaolong maobaolong Dec 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain that why you need the addition condition to judge the value of keyMayExist? @avijayanhwx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants