Test for successful cancellation #70
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
Send `request_cancel` at random and test if it could be cancelled.
kou
reviewed
Apr 5, 2024
Comment on lines
431
to
437
| parser.on("--cancel-rate=RATE", Float, | ||
| "You can specify the rate at which request_cancel is sent." + | ||
| "For example, if you specify 0.3 in this option, " + | ||
| "send request_cancel with the probability of 3/10.", | ||
| "(#{@cancel_rate})") do |rate| | ||
| @cancel_rate = rate | ||
| end |
There was a problem hiding this comment.
これいらないっす。
対象のリクエスト全部をキャンセル対象でいいです。対象を間引くのは既存の--omit-rateでやります。
kou
reviewed
Apr 5, 2024
| def verify_command(groonga1_client, groonga2_client, command) | ||
| command["cache"] = "no" if @options.disable_cache? | ||
| command["cache"] = "no" if @options.verify_performance? | ||
| if @options.cancel_rate > 0 && command.request_id.nil? | ||
| command["request_id"] = generate_request_id.to_s |
kou
reviewed
Apr 5, 2024
| def verify_command(groonga1_client, groonga2_client, command) | ||
| command["cache"] = "no" if @options.disable_cache? | ||
| command["cache"] = "no" if @options.verify_performance? | ||
| if @options.cancel_rate > 0 && command.request_id.nil? |
There was a problem hiding this comment.
@options.verify_cancel?を作ってそれを使おうか。
options.verify_cancel?のときはcommand.request_idにあってもそれを上書きでいいです。
kou
reviewed
Apr 5, 2024
Comment on lines
196
to
199
| response2 = nil | ||
| request = groonga2_client.execute(command) do |response| | ||
| response2 = response | ||
| end |
There was a problem hiding this comment.
↓みたいに@options.verify_cancel?じゃないときは既存のコードでよろしく。
Suggested change
| response2 = nil | |
| request = groonga2_client.execute(command) do |response| | |
| response2 = response | |
| end | |
| if @options.verify_cancel? | |
| response2 = nil | |
| request2 = groonga2_client.execute(command) do |response| | |
| response2 = response | |
| end | |
| # ... | |
| else | |
| response2 = groonga2_client.execute(command) | |
| end |
kou
reviewed
Apr 5, 2024
| response2 = response | ||
| end | ||
| canceled = false | ||
| if rand < @options.cancel_rate |
There was a problem hiding this comment.
キャンセルするかどうかのランダム性はいらなくて、どのくらい後にキャンセルするかのランダム性は欲しい。すぐにキャンセルだと処理の初期のキャンセルしかテストできない。
たとえば↓みたいな感じ。
sleep(rand(0..10.0))
@options.groonga2.create_client do |cancel_client|
# ...
endThere was a problem hiding this comment.
ランダムってそういうことですか。勘違いしてました。
kou
reviewed
Apr 5, 2024
Comment on lines
209
to
212
| if canceled | ||
| if response2.return_code != -77 # GRN_CANCEL | ||
| @events.push([:error, command, 'cancelled and return_code is not GRN_CANCEL(-77)']) | ||
| end |
There was a problem hiding this comment.
request_cancelしたら確実にキャンセルできるわけじゃないからこのチェックはいらない。
@options.verify_cancel?のときはreturn_codeが-77なら結果を比較せずに終了、そうじゃないなら通常の処理、でいいっす。
|
一通り変更しました。 @kou 確認をお願いします。 |
kou
reviewed
Apr 8, 2024
| @verify_cancel = boolean | ||
| end | ||
| parser.on("--cancel-max-wait=SECONDS", Float, | ||
| "Used with `--verify_cancel`." + |
There was a problem hiding this comment.
We can't use Markdown:
Suggested change
| "Used with `--verify_cancel`." + | |
| "Used with --verify_cancel. " + |
| parser.on("--cancel-max-wait=SECONDS", Float, | ||
| "Used with `--verify_cancel`." + | ||
| "You can specify the maximum number of seconds to wait " + | ||
| "before sending `request_cancel`." + |
There was a problem hiding this comment.
Suggested change
| "before sending `request_cancel`." + | |
| "before sending request_cancel command. " + |
| "You can specify the maximum number of seconds to wait " + | ||
| "before sending `request_cancel`." + | ||
| "For example, if you specify 5.0 in this option, " + | ||
| "wait randomly between 0~5.0 seconds before sending `request_cancel`.", |
There was a problem hiding this comment.
Suggested change
| "wait randomly between 0~5.0 seconds before sending `request_cancel`.", | |
| "wait randomly between 0~5.0 seconds before sending request_cancel command.", |
| response1 = groonga1_client.execute(command) | ||
| response2 = groonga2_client.execute(command) | ||
| # `groonga2` is new Groonga. |
There was a problem hiding this comment.
Suggested change
| # `groonga2` is new Groonga. | |
| # groonga2 is new Groonga. |
| end | ||
| request.wait | ||
|
|
||
| return if response2.return_code == -77 # GRN_CANCEL |
kou
reviewed
Apr 8, 2024
| @verify_cancel = boolean | ||
| end | ||
| parser.on("--cancel-max-wait=SECONDS", Float, | ||
| "Used with --verify_cancel." + |
There was a problem hiding this comment.
Suggested change
| "Used with --verify_cancel." + | |
| "Used with --verify_cancel. " + |
| end | ||
|
|
||
| parser.on("--cancel-max-wait=SECONDS", Float, | ||
| "Used with --verify_cancel." + |
There was a problem hiding this comment.
Suggested change
| "Used with --verify_cancel." + | |
| "Used with --verify_cancel. " + |
| parser.on("--cancel-max-wait=SECONDS", Float, | ||
| "Used with --verify_cancel." + | ||
| "You can specify the maximum number of seconds to wait " + | ||
| "before sending request_cancel command." + |
There was a problem hiding this comment.
Suggested change
| "before sending request_cancel command." + | |
| "before sending request_cancel command. " + |
| "You can specify the maximum number of seconds to wait " + | ||
| "before sending request_cancel command." + | ||
| "For example, if you specify 5.0 in this option, " + | ||
| "wait randomly between 0~5.0 seconds before sending `request_cancel`.", |
There was a problem hiding this comment.
Suggested change
| "wait randomly between 0~5.0 seconds before sending `request_cancel`.", | |
| "wait randomly between 0~5.0 seconds before sending request_cancel command.", |
| request = groonga2_client.execute(command) do |response| | ||
| response2 = response | ||
| end | ||
| # Randomize timing of sending `request_cancel` |
There was a problem hiding this comment.
Suggested change
| # Randomize timing of sending `request_cancel` | |
| # Randomize timing of sending request_cancel |
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.
Send
request_cancelat random and test if it could be cancelled.