A mechanism to notify MVC about relevant client library status changes on the cluster #5857
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sfc-gh-vgasiunas
requested review from
xis19,
sfc-gh-abeamon and
sfc-gh-mpilman
and removed request for
xis19
|
Only the last 2 commits are new to this PR, the others are from the previous one. It seems is better to wait until the first PR is merged. |
AWS CodeBuild CI Report for Linux CentOS 7
|
…nts to download and activate a library; Operations to read and change client library status
sfc-gh-vgasiunas
force-pushed
the
notify-client-lib-changes
branch
from
f304b61
to
875824b
Compare
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for macOS Catalina 10.15
|
| @@ -46,7 +47,7 @@ struct ClientLibBinaryInfo { | |||
| #define ASSERT_INDEX_IN_RANGE(idx, arr) ASSERT(idx >= 0 && idx < sizeof(arr) / sizeof(arr[0])) | |||
|
|
|||
| const std::string& getStatusName(ClientLibStatus status) { | |||
| static const std::string statusNames[] = { "disabled", "available", "uploading" }; | |||
| static const std::string statusNames[] = { "disabled", "available", "uploading", "download", "active" }; | |||
There was a problem hiding this comment.
What does the available state represent? Would this be for clients to opt in to download it?
There was a problem hiding this comment.
Available means it is possible to download the library, but the clients are not instructed to download it yet. Maybe we don't need the that state. I introduced it before designing the client-side behavior.
|
|
||
| void updateClientLibChangeCounter(Transaction& tr, ClientLibStatus status) { | ||
| static const int64_t counterIncVal = 1; | ||
| if (status == ClientLibStatus::DOWNLOAD || status == ClientLibStatus::ACTIVE) { |
There was a problem hiding this comment.
Are there states that would trigger a client to unload a library (e.g. if we switched from ACTIVE/DOWNLOAD to DISABLED)?
There was a problem hiding this comment.
A good point. We should also notify about libraries losing "ACTIVE/DOWNLOAD" status or libraries with "ACTIVE/DOWNLOAD" being deleted.
| @@ -489,7 +506,7 @@ ACTOR Future<Void> downloadClientLibrary(Database db, | |||
| } | |||
|
|
|||
| // Allow downloading only libraries in the available state | |||
There was a problem hiding this comment.
This comment is out of date
| @@ -1467,7 +1494,7 @@ Reference<ClientInfo> MultiVersionApi::getLocalClient() { | |||
|
|
|||
| void MultiVersionApi::selectApiVersion(int apiVersion) { | |||
| if (!localClient) { | |||
| localClient = makeReference<ClientInfo>(ThreadSafeApi::api); | |||
| // localClient = makeReference<ClientInfo>(ThreadSafeApi::api); | |||
There was a problem hiding this comment.
If we're getting rid of this, we can get rid of the whole if block.
There was a problem hiding this comment.
Oh, I forgot to change this. It should take IClientApi::localApi here
…s that have upload or active status; Declare library status access and change transactions as lock aware
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
sfc-gh-abeamon
previously approved these changes
Nov 10, 2021
AWS CodeBuild CI Report for macOS Catalina 10.15
|
AWS CodeBuild CI Report for Linux CentOS 7
|
sfc-gh-mpilman
approved these changes
Nov 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing 2 new statuses for uploaded client libraries: "download" and "active". The intention is that the "download" status instructs the clients to download the library, and "active" status to use the library to connect to the server. (The client-side behavior is not implemented yet). Whenever new client libraries are uploaded with one of those statuses, the clients are notified about these changes.
For notification we introduce a global client library change counter. It is maintained as a value the system key space, and is atomically incremented by the transactions that change client library status to "download" or "active". Cluster controller monitors the changes of the counter by monitoring it. It notifies the clients about the change over the ClientDBInfo structure, which is extended with an additional field to store change counter. Currently clients can monitor the changes by waiting on the DatabaseContext::onClientLibStatusChanged future.
ClientLibManagement workload is extended to test notifications after uploads and client library status changes.
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-branchormasterif this is the youngest branch)