privval: increase read/write timeout to 5s and calculate ping interval based on it #5638
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
melekes
requested review from
ebuchman,
erikgrinaker and
tessr
as code owners
This was referenced Nov 10, 2020
Partially closes #5550 Co-authored-by: JoeK <JoeK@not-my.email>
melekes
force-pushed
the
anton/5550-validator-fails-to-sync
branch
from
31cf562
to
ad9b2db
Compare
tac0turtle
reviewed
Nov 10, 2020
|
Modified the code to calculate ping interval based on read/write timeout: |
melekes
changed the title
melekes
force-pushed
the
anton/5550-validator-fails-to-sync
branch
from
c735b53
to
1972448
Compare
Codecov Report
@@ Coverage Diff @@
## master #5638 +/- ##
==========================================
+ Coverage 61.11% 61.14% +0.02%
==========================================
Files 262 262
Lines 23726 23730 +4
==========================================
+ Hits 14501 14510 +9
+ Misses 7770 7767 -3
+ Partials 1455 1453 -2
|
|
@marbar3778 extracted go 1.15 update into a separate PR. |
| @@ -25,6 +36,7 @@ type SignerListenerEndpoint struct { | |||
|
|
|||
| timeoutAccept time.Duration | |||
| pingTimer *time.Ticker | |||
| pingInterval time.Duration | |||
There was a problem hiding this comment.
Do we need this? We're only setting up the timer on construction anyway, so it's not really used for anything.
There was a problem hiding this comment.
I am going to add reset call in the follow up PR
-
// Reset pingTimer to avoid sending unnecessary pings. -
sl.pingTimer.Reset(sl.pingInterval)
Had to remove it because of go 1.15 requirement.
erikgrinaker
approved these changes
Nov 10, 2020
lovincyrus
added a commit
that referenced
this pull request
Dec 4, 2020
* CHANGELOG: update to reflect v0.34.0-rc6 (#5622) Note that this also deletes everything from CHANGELOG_PENDING that was included in RC6. * .github: move mergify config * remove appveyor.yml * remove unused PHILOSOPHY file * scripts: move build.sh into scripts * .github: move codecov.yml into .github * .github: move codecov config into .github * CHANGELOG: add breaking Version name change (#5628) The good folks at Regen pointed out that this was an additional breaking change when they upgraded to use RC6: cosmos/cosmos-sdk#7828 (comment) * remove misbehaviors from e2e generator (#5629) * build(deps): Bump actions/cache from v2.1.2 to v2.1.3 (#5633) Bumps [actions/cache](https://github.com/actions/cache) from v2.1.2 to v2.1.3. - [Release notes](https://github.com/actions/cache/releases) - [Commits](actions/cache@v2.1.2...0781355) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * cmd: add support for --key (#5612) Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> * move broadcast_evidence rpc call from info to evidence (#5634) * Merge PR #5624: Types ValidateBasic Tests * crypto: adopt zip215 ed25519 verification (#5632) * build(deps): Bump google.golang.org/grpc from 1.33.1 to 1.33.2 (#5635) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.33.1 to 1.33.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/grpc/grpc-go/releases">google.golang.org/grpc's releases</a>.</em></p> <blockquote> <h2>Release 1.33.2</h2> <ul> <li>protobuf: update all generated code to google.golang.org/protobuf (<a href="https://github-redirect.dependabot.com/grpc/grpc-go/issues/3932">#3932</a>)</li> <li>xdsclient: populate error details for NACK (<a href="https://github-redirect.dependabot.com/grpc/grpc-go/issues/3975">#3975</a>)</li> <li>internal/credentials: fix a bug and add one more helper function SPIFFEIDFromCert (<a href="https://github-redirect.dependabot.com/grpc/grpc-go/issues/3929">#3929</a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/grpc/grpc-go/commit/56d63285d528c0d33f9bf8c355139d91edc047de"><code>56d6328</code></a> github: remove advancedtls examples test</li> <li><a href="https://github.com/grpc/grpc-go/commit/6396e4b7d7b107f2c61fbefe8bca06c4a78dfe9a"><code>6396e4b</code></a> vet: ignore proto deprecation warnings</li> <li><a href="https://github.com/grpc/grpc-go/commit/0afe9d28d8be06318769e6c1e22632b87f47edbb"><code>0afe9d2</code></a> github: add Github Actions workflow for tests; support in vet.sh (<a href="https://github-redirect.dependabot.com/grpc/grpc-go/issues/4005">#4005</a>)</li> <li><a href="https://github.com/grpc/grpc-go/commit/8a0ca33b8542c02085e3d6988fb6e0678b1a9ed4"><code>8a0ca33</code></a> Change version to 1.33.2</li> <li><a href="https://github.com/grpc/grpc-go/commit/c1989b58a5867364d14a2089eef6bfbbc77ef23b"><code>c1989b5</code></a> protobuf: update all generated code to google.golang.org/protobuf (<a href="https://github-redirect.dependabot.com/grpc/grpc-go/issues/3932">#3932</a>)</li> <li><a href="https://github.com/grpc/grpc-go/commit/b205df69d4fed4ef9b38c0a772f98a86be4b9c87"><code>b205df6</code></a> xdsclient: populate error details for NACK (<a href="https://github-redirect.dependabot.com/grpc/grpc-go/issues/3975">#3975</a>)</li> <li><a href="https://github.com/grpc/grpc-go/commit/75e27683ed73aa11adad2c7520bf89e8b8292fb7"><code>75e2768</code></a> internal/credentials: fix a bug and add one more helper function SPIFFEIDFrom...</li> <li><a href="https://github.com/grpc/grpc-go/commit/17493ac06746ffc550e9b1fec77a158509d2beae"><code>17493ac</code></a> Change version to 1.33.2-dev</li> <li>See full diff in <a href="https://github.com/grpc/grpc-go/compare/v1.33.1...v1.33.2">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/configuring-github-dependabot-security-updates) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> * privval: increase read/write timeout to 5s and calculate ping interval based on it (#5638) Partially closes #5550 * bump go version to 1.15 (#5639) * remove Vagrantfile (#5641) we're no longer using it for development * privval: reset pingTimer to avoid sending unnecessary pings (#5642) Refs #5550 * fix docker deployment (#5647) * build(deps): Bump github.com/tendermint/tm-db from 0.6.2 to 0.6.3 Bumps [github.com/tendermint/tm-db](https://github.com/tendermint/tm-db) from 0.6.2 to 0.6.3. - [Release notes](https://github.com/tendermint/tm-db/releases) - [Changelog](https://github.com/tendermint/tm-db/blob/master/CHANGELOG.md) - [Commits](tendermint/tm-db@v0.6.2...v0.6.3) Signed-off-by: dependabot[bot] <support@github.com> * libs/os: add test case for TrapSignal (#5646) * proto: buf for everything (#5650) ## Description - remove installation of protoc - use buf protoc to generate proto stubs prior to approving could someone test locally. I restarted my docker instance and its been stuck for 20+ minutes Closes: #XXX * libs/os: remove unused aliases, add test cases (#5654) Remove unused ReadFile (unused) and WriteFile (almost unused, alias of ioutil.WriteFile). Add testcases for Must{Read,Write}File. * build(deps): Bump rtCamp/action-slack-notify from e9db0ef to 2.1.1 Bumps [rtCamp/action-slack-notify](https://github.com/rtCamp/action-slack-notify) from e9db0ef to 2.1.1. This release includes the previously tagged commit. - [Release notes](https://github.com/rtCamp/action-slack-notify/releases) - [Commits](rtCamp/action-slack-notify@e9db0ef...ecc1353) Signed-off-by: dependabot[bot] <support@github.com> * go.mod: upgrade iavl and deps (#5657) Bumps IAVL, which pulled in some other upgrades as well. I think they should be fine though. * remove unused version struct (#5656) * rpc: fix content-type header (#5661) * test/e2e: fix secp failures (#5649) * docs/architecture: add missing ADRs to README, update status of ADR 034 (#5663) * docs: add P2P architecture ADR (#5637) [Rendered](https://github.com/tendermint/tendermint/blob/erik/adr-p2p-architecture/docs/architecture/adr-062-p2p-architecture.md) ADR for the new P2P architecture and abstractions. Related to #2067. * privval: duplicate SecretConnection from p2p package (#5672) This is so that the `privval` package will not be affected when we refactor `p2p` (#5670). We will be migrating to gRPC shortly (#4698). * buf: modify buf.yml, add buf generate (#5653) * ci: remove `add-path` (#5674) * build(deps): Bump codecov/codecov-action from v1.0.14 to v1.0.15 (#5676) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from v1.0.14 to v1.0.15. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Commits](codecov/codecov-action@v1.0.14...239febf) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * light: make fraction parts uint64, ensuring that it is always positive (#5655) * e2e: use ed25519 for secretConn (remote signer) (#5678) ## Description Hardcode ed25519 to dialTCPFn in e2e tests. I will backport `DefaultRequestHandler` fixes This will be replaced when grpc is implemented. * docs: warn developers about calling blocking funcs in Receive (#5679) Refs #2888 * p2p: remove unused MakePoWTarget() (#5684) * upgrading: update 0.34 instructions with updates since RC4 (#5685) Co-authored-by: Marko <marbar3778@yahoo.com> * light: ensure required header fields are present for verification (#5677) * scripts: make linkifier default to 'pull' rather than 'issue' (#5689) * .vscode: remove directory (#5626) * .gitignore: sort (#5690) * changelog: squash changelog from 0.34 RCs into one (#5691) "Squashes" the changelog from RCs 2-6 into one changelog message for 0.34.0, and adds the changelog pending. * .goreleaser: add windows, remove arm (32 bit) (#5692) This updates the goreleaser tooling to build the same binaries that were built for the 0.34.0 release. * CONTRIBUTING: update to match the release flow used for 0.34.0 (#5697) * codecov: validate codecov.yml (#5699) ## Description _Please add a description of the changes that this PR introduces and the files that are the most critical to review._ Closes: #XXX * blockchain/v1: handle peers without blocks (#5701) Closes #5444 Now we record the fact that a peer does not have a requested block and later use this information to make a new request for the same block from another peer. * test/e2e: enable v1 and v2 blockchains (#5702) * test/e2e: enable v1 and v2 blockchains * modify networks/ci.toml * docs: add nodes section (#5604) ## Description - separate docs related to running nodes into the nodes dir. - keep old files but dont display them - bring over debugging like a pro blog Closes: #XXX * build(deps): Bump github.com/cosmos/iavl from 0.15.0-rc5 to 0.15.0 (#5708) Bumps [github.com/cosmos/iavl](https://github.com/cosmos/iavl) from 0.15.0-rc5 to 0.15.0. - [Release notes](https://github.com/cosmos/iavl/releases) - [Changelog](https://github.com/cosmos/iavl/blob/master/CHANGELOG.md) - [Commits](cosmos/iavl@v0.15.0-rc5...v0.15.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * crypto: fix infinite recursion in Secp256k1 string formatting (#5707) This caused stack overflow panics in E2E tests, e.g.: ``` 2020-11-24T02:37:17.6085640Z �[35mvalidator04 |�[0m runtime: goroutine stack exceeds 1000000000-byte limit 2020-11-24T02:37:17.6087818Z �[35mvalidator04 |�[0m runtime: sp=0xc0234b23c0 stack=[0xc0234b2000, 0xc0434b2000] 2020-11-24T02:37:17.6088920Z �[35mvalidator04 |�[0m fatal error: stack overflow 2020-11-24T02:37:17.6089776Z �[35mvalidator04 |�[0m 2020-11-24T02:37:17.6090569Z �[35mvalidator04 |�[0m runtime stack: 2020-11-24T02:37:17.6091677Z �[35mvalidator04 |�[0m runtime.throw(0x12dc476, 0xe) 2020-11-24T02:37:17.6093123Z �[35mvalidator04 |�[0m /usr/local/go/src/runtime/panic.go:1116 +0x72 2020-11-24T02:37:17.6094320Z �[35mvalidator04 |�[0m runtime.newstack() 2020-11-24T02:37:17.6095374Z �[35mvalidator04 |�[0m /usr/local/go/src/runtime/stack.go:1067 +0x78d 2020-11-24T02:37:17.6096381Z �[35mvalidator04 |�[0m runtime.morestack() 2020-11-24T02:37:17.6097657Z �[35mvalidator04 |�[0m /usr/local/go/src/runtime/asm_amd64.s:449 +0x8f 2020-11-24T02:37:17.6098505Z �[35mvalidator04 |�[0m 2020-11-24T02:37:17.6099328Z �[35mvalidator04 |�[0m goroutine 88 [running]: 2020-11-24T02:37:17.6100470Z �[35mvalidator04 |�[0m runtime.heapBitsSetType(0xc009565380, 0x20, 0x18, 0x1137e00) 2020-11-24T02:37:17.6101961Z �[35mvalidator04 |�[0m /usr/local/go/src/runtime/mbitmap.go:911 +0xaa5 fp=0xc0234b23d0 sp=0xc0234b23c8 pc=0x432625 2020-11-24T02:37:17.6103906Z �[35mvalidator04 |�[0m runtime.mallocgc(0x20, 0x1137e00, 0x117b601, 0x11e9240) 2020-11-24T02:37:17.6105179Z �[35mvalidator04 |�[0m /usr/local/go/src/runtime/malloc.go:1090 +0x5a5 fp=0xc0234b2470 sp=0xc0234b23d0 pc=0x428b25 2020-11-24T02:37:17.6106540Z �[35mvalidator04 |�[0m runtime.convTslice(0xc002743710, 0x21, 0x21, 0xc0234b24e8) 2020-11-24T02:37:17.6107861Z �[35mvalidator04 |�[0m /usr/local/go/src/runtime/iface.go:385 +0x59 fp=0xc0234b24a0 sp=0xc0234b2470 pc=0x426379 2020-11-24T02:37:17.6109315Z �[35mvalidator04 |�[0m github.com/tendermint/tendermint/crypto/secp256k1.PubKey.String(...) 2020-11-24T02:37:17.6151692Z �[35mvalidator04 |�[0m /src/tendermint/crypto/secp256k1/secp256k1.go:161 2020-11-24T02:37:17.6153872Z �[35mvalidator04 |�[0m github.com/tendermint/tendermint/crypto/secp256k1.(*PubKey).String(0xc009565360, 0x11e9240, 0xc009565360) 2020-11-24T02:37:17.6157421Z �[35mvalidator04 |�[0m <autogenerated>:1 +0x65 fp=0xc0234b24f8 sp=0xc0234b24a0 pc=0x656965 2020-11-24T02:37:17.6159134Z �[35mvalidator04 |�[0m fmt.(*pp).handleMethods(0xc00956c680, 0x58, 0xc0234b2801) 2020-11-24T02:37:17.6161462Z �[35mvalidator04 |�[0m /usr/local/go/src/fmt/print.go:630 +0x30a fp=0xc0234b2768 sp=0xc0234b24f8 pc=0x518b8a [...] 2020-11-24T02:37:17.6649685Z �[35mvalidator04 |�[0m /usr/local/go/src/fmt/print.go:630 +0x30a fp=0xc0234b7f48 sp=0xc0234b7cd8 pc=0x518b8a 2020-11-24T02:37:17.6651177Z �[35mvalidator04 |�[0m created by github.com/tendermint/tendermint/node.startStateSync 2020-11-24T02:37:17.6652521Z �[35mvalidator04 |�[0m /src/tendermint/node/node.go:587 +0x150 ``` * version: add abci version to handshake (#5706) ## Description - add `AbciVersion` RequestInfo Closes: #2804 * test: fix TestByzantinePrevoteEquivocation flake (#5710) This fixes spurious `TestByzantinePrevoteEquivocation` failures by extending the block range and time spent waiting for evidence. I've seen many runs where the evidence isn't committed until e.g. height 27. Haven't looked into _why_ this happens, but as long as the evidence is committed eventually and the test doesn't spuriously fail I'm (mostly) happy. WDYT @cmwaters? * README: update link to Tendermint blog (#5713) * ci: remove circle (#5714) * remove circle * remove prefix * ci: build for 32 bit, libs: fix overflow (#5700) * use Cleanup(),TempDir() in test cases (#5723) Replace defer with t.Cleanup(). Replace the combination of ioutil.TempDir, error checking and defer os.RemoveAll() with Go testing.T's new TempDir() helper. Mark auxiliary functions as test helpers. * abci: modify Client interface and socket client (#5673) `abci.Client`: - Sync and Async methods now accept a context for cancellation * grpc client uses context to cancel both Sync and Async requests * local client ignores context parameter * socket client uses context to cancel Sync requests and to drop Async requests before sending them if context was cancelled prior to that - Async methods return an error * socket client returns an error immediately if queue is full for Async requests * local client always returns nil error * grpc client returns an error if context was cancelled before we got response or the receiving queue had a space for response (do not confuse with the sending queue from the socket client) - specify clients semantics in [doc.go](https://raw.githubusercontent.com/tendermint/tendermint/27112fffa62276bc016d56741f686f0f77931748/abci/client/doc.go) `mempool.TxInfo` - add optional `Context` to `TxInfo`, which can be used to cancel `CheckTx` request Closes #5190 * readme: remover circleci badge (#5729) ## Description - remove circleci badge from readme Closes: #XXX * libs/bits: validate BitArray in FromProto (#5720) Closes #5705 * light: minor fixes / standardising errors (#5716) ## Description I'm just doing a self audit of the light client. There's a few things I've changed - Validate trust level in `VerifyNonAdjacent` function - Make errNoWitnesses public (it's something people running software on top of a light client should be able to parse) - Remove `ChainID` check of witnesses on start up. We do this already when we compare the first header with witnesses - Remove `ChainID()` from provider interface Closes: #4538 * blockchain/v1: fix deadlock (#5711) I introduced a new variable - syncEnded, which is now used to prevent sending new events to channels (which would block otherwise) if reactor is finished syncing Closes #4591 * build(deps): Bump vuepress-theme-cosmos from 1.0.175 to 1.0.176 in /docs (#5727) Bumps [vuepress-theme-cosmos](https://github.com/cosmos/vuepress-theme-cosmos) from 1.0.175 to 1.0.176. - [Release notes](https://github.com/cosmos/vuepress-theme-cosmos/releases) - [Commits](https://github.com/cosmos/vuepress-theme-cosmos/commits) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * p2p/pex: fix flaky tests (#5733) *testing.T.TempDir() causes test cases to fail when it is unable to remove the temporary directory once the test case execution terminates. This seems to happen often with pex reactor test cases. * consensus: fix flaky tests (#5734) Replace testing.T.Cleanup() with deferred function calls in test helpers as those cleanup functions need to be called once the helper returns and not when the entire test ends. This reverts a few of the changes introduced in #5723. Thanks: @erikgrinaker for pointing this out. Ref: #5732 * blockchain/v1: remove in favor of v2 (#5728) * changelog: add entry back (#5738) ## Description add back removed changelog entry Closes: #XXX * UX: version configuration (#5740) ## Description - when not on a tag `tendermint version` will return "unreleased-`branchName`-`commitHash`" Closes: #XXX * build(deps): Bump google.golang.org/grpc from 1.33.2 to 1.34.0 (#5737) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Marko <marbar3778@yahoo.com> * reactors: omit incoming message bytes from reactor logs (#5743) After a reactor has failed to parse an incoming message, it shouldn't output the "bad" data into the logs, as that data is unfiltered and could have anything in it. (We also don't think this information is helpful to have in the logs anyways.) * evidence: omit bytes field (#5745) Follow-up to #5743 * contributing: simplify our minor release process (#5749) * build(deps): Bump vuepress-theme-cosmos from 1.0.176 to 1.0.177 in /docs (#5746) Bumps [vuepress-theme-cosmos](https://github.com/cosmos/vuepress-theme-cosmos) from 1.0.176 to 1.0.177. <details> <summary>Commits</summary> <ul> <li>See full diff in <a href="https://github.com/cosmos/vuepress-theme-cosmos/commits">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/configuring-github-dependabot-security-updates) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Co-authored-by: Tess Rinearson <tess.rinearson@gmail.com> Co-authored-by: Callum Waters <cmwaters19@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Erik Grinaker <erik@interchain.berlin>
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.
Partially closes #5550