-
Notifications
You must be signed in to change notification settings - Fork 96
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
Add condition
option to local queries
#450
Merged
Merged
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
After submitting the pull request, I admit the parameter name Here is what the call looks like: ra:local_query(ServerId, QueryFun, #{limit => {Index, Term}}). What about ra:local_query(ServerId, QueryFun, #{wait_for_index => {Index, Term}}). Any opinion? |
dumbbell
force-pushed
the
add-local-query-limit-option
branch
from
June 20, 2024 15:26
8764db6
to
95b3629
Compare
dumbbell
force-pushed
the
add-local-query-limit-option
branch
from
June 24, 2024 13:32
95b3629
to
820cd0f
Compare
dumbbell
changed the title
Add
Add Jun 24, 2024
limit
option to local querieswait_for_index
option to local queries
I renamed the option from |
dumbbell
force-pushed
the
add-local-query-limit-option
branch
from
June 24, 2024 13:50
820cd0f
to
f4211b5
Compare
dumbbell
force-pushed
the
add-local-query-limit-option
branch
from
June 25, 2024 14:28
f4211b5
to
10a1f9d
Compare
kjnilsson
requested changes
Jun 26, 2024
dumbbell
force-pushed
the
add-local-query-limit-option
branch
2 times, most recently
from
June 26, 2024 13:29
f340066
to
7bfec04
Compare
kjnilsson
changed the title
Add
Add Jun 26, 2024
wait_for_index
option to local queriescondition
option to local queries
dumbbell
force-pushed
the
add-local-query-limit-option
branch
from
June 26, 2024 13:51
7bfec04
to
f3a984b
Compare
[Why] It allows to wait for a condition to become true before the query can be executed. The condition we need right now is to wait for an index to be applied locally (or on the leader). It is useful when the caller wants to be sure that the result of the previous command is "visible" by the next query. By default, it's not guarantied because the command will be considered successfully applied as long as a quorum of Ra servers applied it. This list of Ra servers may not include the local node for instance. [How] If the `condition` option is specified with a `{applied, {Index, Term}}` tuple, the query will be evaluated right away if that index is already applied, or it will be added to a list of pending queries. Pending queries are evaluated after each applied batch of commands by the local node. If a pending query's target index was reached or passed, it is evaluated. If a pending query's target term ended, an error is returned. Note that pending queries that timed out from the callers' point of view will still be evaluated once their associated condition becomes true. The reply will be discarded by Erlang however because the process alias will be inactivate at that point. Here is an example: ra:local_query(ServerId, QueryFun, #{condition => {applied, {Index, Term}}}). The `local_query` tuple sent to the Ra server changes format. The old one was: {local_query, QueryFun} The new one is: {local_query, QueryFun, Options} If the remote Ra server that receives the query runs a version of Ra older than the one having this change and thus doesn't understand the new tuple, it will ignore and drop the query. This will lead to a timeout of the query, or an indefinitely hanging call if the timeout was set to `infinity`. Note in the opposite situation, i.e. if a Ra server that knows the new query tuple receives an old tuple, it will evaluate the query as if the options was an empty map. V2: Rename the option from `limit` to `wait_for_index` which is more explicit. V3: Rename the option back to `limit`. It allows to pass other types of condition in the future. Also change the place where pending queries are evaluated. This allows to get rid of the `applied_to` effect. V4: Rename the option to `condition` to make its purpose more intuitive. The value was changed to `{applied, {Index, Term}` to give more meaning to what the condition does. While here, the `ra_idxterm()` type is aliased to `idxterm()` and exported as `ra:idxterm()`.
dumbbell
force-pushed
the
add-local-query-limit-option
branch
from
June 26, 2024 15:03
f3a984b
to
a414eb5
Compare
kjnilsson
approved these changes
Jun 26, 2024
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.
Why
It allows to wait for a condition to become true before the query can be executed.
The condition we need right now is to wait for an index to be applied locally (or on the leader). It is useful when the caller wants to be sure that the result of the previous command is "visible" by the next query.
By default, it's not guarantied because the command will be considered successfully applied as long as a quorum of Ra servers applied it. This list of Ra servers may not include the local node for instance.
How
If the
condition
option is specified with a{applied, {Index, Term}}
tuple, the query will be evaluated right away if that index is already applied, or it will be added to a list of pending queries.Pending queries are evaluated after each applied batch of commands by the local node. If a pending query's target index was reached or passed, it is evaluated. If a pending query's target term ended, an error is returned.
Note that pending queries that timed out from the callers' point of view will still be evaluated once their associated condition becomes true. The reply will be discarded by Erlang however because the process alias will be inactivate at that point.
Here is an example:
The
local_query
tuple sent to the Ra server changes format. The old one was:The new one is:
If the remote Ra server that receives the query runs a version of Ra older than the one having this change and thus doesn't understand the new tuple, it will ignore and drop the query. This will lead to a timeout of the query, or an indefinitely hanging call if the timeout was set to
infinity
.Note in the opposite situation, i.e. if a Ra server that knows the new query tuple receives an old tuple, it will evaluate the query as if the options was an empty map.
V2: Rename the option from
limit
towait_for_index
which is more explicit.V3: Rename the option back to
limit
. It allows to pass other types of condition in the future. Also change the place where pending queries are evaluated. This allows to get rid of theapplied_to
effect.V4: Rename the option to
condition
to make its purpose more intuitive. The value was changed to{applied, {Index, Term}
to give more meaning to what the condition does. While here, thera_idxterm()
type is aliased toidxterm()
and exported asra:idxterm()
.