E52E [BTS-491][3.7] ArangoRestore connect Retry by maierlars · Pull Request #14437 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

[BTS-491][3.7] ArangoRestore connect Retry #14437

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 5, 2021
Merged

[BTS-491][3.7] ArangoRestore connect Retry #14437

merged 6 commits into from
Jul 5, 2021

Conversation

@maierlars
Copy link
Contributor
@maierlars maierlars commented Jul 1, 2021

Scope & Purpose

Oasis found that a connect from arangorestore can timeout and terminate the process. A simple retry solves the issue. Number of retries are configurable and default to 3.

  • 💩 Bugfix (requires CHANGELOG entry)
  • 🍕 New feature (requires CHANGELOG entry, feature documentation and release notes)
  • 🔥 Performance improvement
  • 🔨 Refactoring/simplification
  • 📖 CHANGELOG entry made

Backports:

  • No backports required
  • Backports required for: 3.7, 3.8, devel

Testing & Verification

  • This change is a trivial rework / code cleanup without any test coverage.
  • The behavior in this PR was manually tested
  • This change is already covered by existing tests, such as (please describe tests).
  • This PR adds tests that were used to verify all changes:
    • Added new C++ Unit tests
    • Added new integration tests (e.g. in shell_server / shell_server_aql)
    • Added new resilience tests (only if the feature is impacted by failovers)
  • There are tests in an external testing repository:
  • I ensured this code runs with ASan / TSan or other static verification tools

@maierlars maierlars self-assigned this Jul 1, 2021
@maierlars
Copy link
Contributor Author
maierlars commented Jul 1, 2021

https://jenkins.arangodb.biz/job/arangodb-matrix-pr/16014/

dothebart
dothebart reviewed Jul 1, 2021
View reviewed changes
@maierlars
Copy link
Contributor Author
maierlars commented Jul 1, 2021

https://jenkins.arangodb.biz/job/arangodb-matrix-pr/16018/

@maierlars
Copy link
Contributor Author
maierlars commented Jul 1, 2021

https://jenkins.arangodb.biz/job/arangodb-matrix-pr/16028/

@maierlars
Copy link
Contributor Author
maierlars commented Jul 2, 2021

https://jenkins.arangodb.biz/job/arangodb-matrix-pr/16042/

jsteemann
jsteemann reviewed Jul 2, 2021
View reviewed changes
jsteemann
jsteemann reviewed Jul 2, 2021
View reviewed changes
jsteemann
jsteemann approved these changes Jul 2, 2021
View reviewed changes
Copy link
Contributor
@jsteemann jsteemann left a comment

Choose a reason for hiding this comment

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

LGTM!

@jsteemann
Apply suggestions from code review
23a467b 
Co-authored-by: Jan <jsteemann@users.noreply.github.com>
@maierlars
Copy link
Contributor Author
maierlars commented Jul 2, 2021

https://jenkins.arangodb.biz/job/arangodb-matrix-pr/16052/

This was referenced Jul 5, 2021
Merged
Merged
@KVS85 KVS85 added the 3 ImportExport Export / Import / Dump / Restore / Backup label Jul 5, 2021
@KVS85 KVS85 added this to the 3.7 milestone Jul 5, 2021
@KVS85 KVS85 merged commit 999044c into 3.7 Jul 5, 2021
@KVS85 KVS85 deleted the bug-fix-3.7/restore-connect-retry branch July 5, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dothebart dothebart dothebart left review comments

+1 more reviewer

@jsteemann jsteemann jsteemann approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@maierlars maierlars

Labels

3 ImportExport Export / Import / Dump / Restore / Backup

Projects

None yet

Milestone

3.7

Development

Successfully merging this pull request may close these issues.

5 participants

@maierlars @jsteemann @dothebart @KVS85
0