-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
KeyValueStoreRocksDB histograms to track latencies #6149
Conversation
b940f21
to
eb1d115
Compare
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
eb1d115
to
deabed2
Compare
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for macOS Catalina 10.15
|
e85896e
to
eec774b
Compare
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
eec774b
to
c30bd4e
Compare
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
c30bd4e
to
6328caa
Compare
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
rocksdb::PinnableSlice value; | ||
auto options = getReadOptions(); | ||
uint64_t deadlineMircos = | ||
db->GetEnv()->NowMicros() + (readValueTimeout - (timer_monotonic() - a.startTime)) * 1000000; | ||
uint64_t deadlineMircos = db->GetEnv()->NowMicros() + (readValueTimeout - (now() - a.startTime)) * 1000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why changing to now()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally I have decided to use timer_monotonic for measuring the histograms.
Simulation: timer_monotonic is calling timer() which is 0.1 seconds ahead of now(). Might not show exact latencies, but it should be fine since simulation test are not used for perf measurements.
Non-simulation: timer_monotonic() returns a high precision monotonic clock. Will show exact latencies and expensive, so currently sampling with a knob at the rate of 1 out of 1000 read ops.
Sampling is not perfect but I could not find a better way.
rocksdb::PinnableSlice value; | ||
auto options = getReadOptions(); | ||
uint64_t deadlineMircos = | ||
db->GetEnv()->NowMicros() + (readValueTimeout - (timer_monotonic() - a.startTime)) * 1000000; | ||
uint64_t deadlineMircos = db->GetEnv()->NowMicros() + (readValueTimeout - (now() - a.startTime)) * 1000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
auto s = db->Get(options, db->DefaultColumnFamily(), toSlice(a.key), &value); | ||
readValueGetHistogram->sampleSeconds(now() - dbGetBeginTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to move this to the below if(s.ok()) scope, to calculate only the successful reads?
6328caa
to
995bf25
Compare
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
995bf25
to
dc8ba37
Compare
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
fdbclient/ServerKnobs.cpp
Outdated
@@ -354,6 +354,7 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi | |||
init( ROCKSDB_READ_QUEUE_SOFT_MAX, 500 ); | |||
init( ROCKSDB_FETCH_QUEUE_HARD_MAX, 100 ); | |||
init( ROCKSDB_FETCH_QUEUE_SOFT_MAX, 50 ); | |||
init( ROCKSDB_HISTOGRAMS_SAMPLE_RATE, 0.001 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BUGGIFY the knob to -1 to cover the code path when this histogram is disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
What's the sim test result after resolving the comments? |
dc8ba37
to
1f30368
Compare
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
20211222-071327-neethuhaneeshabingi-e8449ab7ef9f700e ended=100000 fail=4 |
KeyValueStoreRocksDB histograms to track latencies
KeyValueStoreRocksDB histograms to track latencies
KeyValueStoreRocksDB histograms to track latencies
Code-Reviewer Section
The general guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branch
ormaster
if this is the youngest branch)