Skip to content
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

Improve reliability and logging of setup node. #381

Merged
merged 6 commits into from
Jun 8, 2020

Conversation

evanlinjin
Copy link
Contributor

@evanlinjin evanlinjin commented May 31, 2020

Fixes #379

Changes:

  • Split setup node into multiple functions for easier testability.
  • Check inputs for each setup request.
  • Use context.Context in all parts of the setup process to ensure we cancel on hard timeout.
  • Introduced routerclient.Map to clean up various parts of the logic, as well as allow the setup process to be speedier.
  • Got rid of the routing.Path data type as it is confusing. I has resorted to use []routing.Hop directly.
  • Unit tests.

How to test this PR:

  • make check
  • I need to update skywire-services interactive env to work with these changes.

@evanlinjin evanlinjin changed the base branch from master to develop May 31, 2020 06:35
* Split setup node into multiple functions for easier testability.
* Check inputs for each setup request.
* Use context.Context in all parts of the setup process to ensure we cancel on hard timeout.
* Introduced routerclient.Map to clean up various parts of the logic, as well as allow the setup process to be speedier.
* Got rid of the routing.Path data type as it is confusing. I has resorted to use []routing.Hop directly.
* Unit tests.
@evanlinjin evanlinjin force-pushed the fix/setup-node-hangs branch from 0c0c05c to 82263cd Compare June 1, 2020 11:51
@evanlinjin evanlinjin changed the title WIP: Setup node troubleshooting and fixing. Improve reliability and logging of setup node. Jun 1, 2020
@evanlinjin evanlinjin marked this pull request as ready for review June 1, 2020 12:48
@nkryuchkov nkryuchkov self-requested a review June 2, 2020 08:34
Copy link
Contributor

@nkryuchkov nkryuchkov left a comment

Choose a reason for hiding this comment

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

Great improvements!

.travis.yml Show resolved Hide resolved
pkg/setup/node_old_test.go Outdated Show resolved Hide resolved
pkg/setup/node_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nkryuchkov nkryuchkov left a comment

Choose a reason for hiding this comment

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

Great job! Just a few minor requests.

pkg/setup/id_reserver_test.go Show resolved Hide resolved
pkg/setup/node_test.go Show resolved Hide resolved
pkg/setup/node_test.go Show resolved Hide resolved
pkg/routing/route.go Show resolved Hide resolved
Actual dialing to the snettest.Env struct has also been temporarily removed. This will be addressed in #395
@nkryuchkov nkryuchkov merged commit 0b19237 into develop Jun 8, 2020
@jdknives jdknives mentioned this pull request Jun 8, 2020
@jdknives jdknives deleted the fix/setup-node-hangs branch June 15, 2020 17:35
jdknives pushed a commit that referenced this pull request Oct 19, 2020
Improve reliability and logging of setup node.

Former-commit-id: 0b19237
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Troubleshoot setup node.
2 participants