Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix get rocksdb ops tcl test case #668

Merged
merged 4 commits into from
Jun 26, 2022
Merged

Conversation

caipengbo
Copy link
Contributor

fix #658

@caipengbo caipengbo requested review from git-hulk and tisonkun June 25, 2022 07:33
@git-hulk
Copy link
Member

As commented in #658 (comment), seems our current ops comparison is not so determined since it depends on the server cron to record metrics, also ops will be changed by the time slot. I think we can change the assert condition to guarantee those ops metrics can't be zero. How do you think?

@caipengbo
Copy link
Contributor Author

caipengbo commented Jun 26, 2022

I think we can change the assert condition to guarantee those ops metrics can't be zero.

Yes, this operation is the metric from rocksdb, which should return as correct as possible.

And how about using put ops compare to other ops?

@git-hulk
Copy link
Member

I think we can change the assert condition to guarantee those ops metrics can't be zero.

Yes, this operation is the metric from rocksdb, which should return as correct as possible.

And how about using put ops compare to other ops?

For those ops metrics would be affected by other test cases if they have many get/put/seek operations, coz our metrics calculate range is 16s. Can see: https://github.com/apache/incubator-kvrocks/blob/0b2de8ab6f661d90a279a0c11096755d7733b11c/src/stats.cc#L108

I think it's ok to check ops > 0 only for current implementation, we should export those metrics counter if want to check them more precisely.

@git-hulk git-hulk merged commit ecf95c9 into apache:unstable Jun 26, 2022
jackwener pushed a commit to jackwener/incubator-kvrocks that referenced this pull request Jul 1, 2022
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.

get rocksdb ops by COMMAND INFO in tests/unit/command.tcl [expr abs($cmd_qps - $put_qps)] < 10
2 participants