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

Improve proxy GRV latencies and responsiveness #2735

Merged
merged 6 commits into from
Apr 14, 2020

Conversation

ajbeamon
Copy link
Contributor

This changes the algorithm used to decide whether a proxy can respond to a start request. The previous algorithm was prone to introducing unneeded latency as the request rate went up, and it could be very slow to respond to changes in workload that would allow lower priority work to start.

The new algorithm has the proxy tracking a smoothed rate for how many transactions can be started and how many have been started, permitting transactions to be started if the rate difference times some time window is larger than the number to be started. It also keeps a budget to allow it to start transaction batches that are larger than can be handled by the windowed mechanism.

Resolves #2206

…ve performance and increase responsiveness to changes in workload.
@ajbeamon
Copy link
Contributor Author

An example of the type of performance improvement:

When running a test at about 90% of ratekeeper's limit, the new algorithm had 99th percentile latencies around 1-2ms, while the old algorithm was around 75ms.

@etschannen etschannen self-assigned this Feb 26, 2020
Copy link
Contributor

@mpilman mpilman left a comment

Choose a reason for hiding this comment

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

This is quite clever and I think this should be strictly better than the old mechanism. However I would like to see some comments as in order to understand this one has to read through quite a lot of code.

I would also suggest you add your explanation that you put into this PR as a comment into the code. Maybe on top of TransactionRateInfo? That will make it a lot easier to understand this in the future.

fdbserver/MasterProxyServer.actor.cpp Outdated Show resolved Hide resolved
fdbserver/MasterProxyServer.actor.cpp Show resolved Hide resolved
break;
} else if (req.priority() < GetReadVersionRequest::PRIORITY_SYSTEM_IMMEDIATE &&
!normalRateInfo.canStart(transactionsStarted[0] + transactionsStarted[1])) {
if(req.priority() < GetReadVersionRequest::PRIORITY_DEFAULT && !batchRateInfo.canStart(transactionsStarted[0] + transactionsStarted[1], tc)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this PR has not applied clang-format.
I also saw some trailing space after checking it out locally.
Would you mind clang-formatting the change?

…y-perf-improvements

# Conflicts:
#	fdbserver/Knobs.cpp
#	fdbserver/Knobs.h
#	fdbserver/MasterProxyServer.actor.cpp
@etschannen etschannen merged commit dd951af into apple:master Apr 14, 2020
@ajbeamon ajbeamon deleted the grv-proxy-perf-improvements branch May 19, 2020 15:34
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.

Proxy GRV budgeting performance improvements
4 participants