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

[Feat] Add the number of connected peers to Status #1576

Merged
merged 9 commits into from
Nov 30, 2021

Conversation

s8sato
Copy link
Contributor

@s8sato s8sato commented Nov 3, 2021

Description of the Change

  • Number of connected peers, except for the reporting peer itself

Screenshot from 2021-11-16 01-54-03

Issue

Closes #1387

Benefits

Possible Drawbacks

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Nov 3, 2021
@codecov
Copy link

codecov bot commented Nov 3, 2021

Codecov Report

Merging #1576 (5e2403b) into iroha2-dev (abdc590) will decrease coverage by 3.89%.
The diff coverage is 72.96%.

Impacted file tree graph

@@              Coverage Diff               @@
##           iroha2-dev    #1576      +/-   ##
==============================================
- Coverage       77.07%   73.18%   -3.90%     
==============================================
  Files             122      123       +1     
  Lines           20141    21621    +1480     
==============================================
+ Hits            15524    15823     +299     
- Misses           4617     5798    +1181     
Impacted Files Coverage Δ
client/tests/tests/multiple_blocks_created.rs 0.00% <0.00%> (ø)
client_cli/src/main.rs 0.27% <0.00%> (-0.14%) ⬇️
core/src/smartcontracts/isi/mod.rs 74.47% <ø> (-7.24%) ⬇️
data_model/src/lib.rs 59.28% <ø> (-7.15%) ⬇️
p2p/src/lib.rs 47.82% <ø> (+13.04%) ⬆️
core/test_network/src/lib.rs 65.57% <70.58%> (-14.84%) ⬇️
p2p/src/network.rs 65.48% <82.50%> (-13.47%) ⬇️
p2p/tests/p2p.rs 82.86% <82.60%> (-14.96%) ⬇️
core/src/torii/mod.rs 70.82% <84.84%> (-17.84%) ⬇️
core/src/sumeragi/mod.rs 72.04% <87.50%> (-17.43%) ⬇️
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abdc590...5e2403b. Read the comment docs.

@s8sato s8sato force-pushed the feat/1387 branch 2 times, most recently from 57260c9 to 0c43641 Compare November 8, 2021 12:13
@s8sato s8sato marked this pull request as ready for review November 8, 2021 12:16
@s8sato s8sato added api-changes Changes in the API for client libraries metrics labels Nov 8, 2021
@s8sato s8sato linked an issue Nov 8, 2021 that may be closed by this pull request
@s8sato s8sato requested a review from mversic as a code owner November 9, 2021 06:04
@appetrosyan appetrosyan self-assigned this Nov 9, 2021
core/src/torii/mod.rs Outdated Show resolved Hide resolved
core/src/torii/mod.rs Outdated Show resolved Hide resolved
core/src/torii/mod.rs Outdated Show resolved Hide resolved
@e-ivkov
Copy link
Contributor

e-ivkov commented Nov 9, 2021

Also as suggested by Sofaine let's rename the endpoint to status.

@appetrosyan appetrosyan changed the title Add endpoint for internal metrics for administration [Feat] Add endpoint for internal metrics for administration Nov 11, 2021
@s8sato s8sato changed the title [Feat] Add endpoint for internal metrics for administration Add /status endpoint on specific port Nov 15, 2021
@s8sato s8sato force-pushed the feat/1387 branch 2 times, most recently from 77c8461 to 5a9b748 Compare November 17, 2021 08:25
@s8sato s8sato marked this pull request as draft November 29, 2021 03:49
client_cli/src/main.rs Outdated Show resolved Hide resolved
core/src/sumeragi/mod.rs Show resolved Hide resolved
core/src/torii/mod.rs Outdated Show resolved Hide resolved
core/src/torii/mod.rs Outdated Show resolved Hide resolved
core/src/torii/mod.rs Outdated Show resolved Hide resolved
core/src/torii/mod.rs Outdated Show resolved Hide resolved
data_model/src/lib.rs Outdated Show resolved Hide resolved
docs/source/references/api_spec.md Outdated Show resolved Hide resolved
p2p/src/network.rs Show resolved Hide resolved
p2p/src/network.rs Outdated Show resolved Hide resolved
@appetrosyan appetrosyan changed the title Add /status endpoint to a specific port [Feat] Add /status endpoint to a specific port Nov 29, 2021
@appetrosyan appetrosyan changed the title [Feat] Add /status endpoint to a specific port [Feat] Add /status endpoint to a specific port Nov 29, 2021
@appetrosyan appetrosyan changed the title [Feat] Add /status endpoint to a specific port [Feat] Add /status endpoint to a specific port Nov 29, 2021
@s8sato s8sato marked this pull request as ready for review November 29, 2021 08:56
@e-ivkov
Copy link
Contributor

e-ivkov commented Nov 29, 2021

What do you think about splitting this PR into 2 separate PRs:

  1. Introduce a status endpoint only with number of committed blocks.
  2. Add number of connected peers feature and corrections to p2p that you want to introduce.

I think we will be able to merge the 1st one with minor corrections almost instantly. Then you will have time to work on the second one in detail.

@s8sato s8sato changed the title [Feat] Add /status endpoint to a specific port Add the number of connected peers to Status Nov 30, 2021
@s8sato s8sato marked this pull request as draft November 30, 2021 07:04
@s8sato s8sato marked this pull request as ready for review November 30, 2021 10:38
@s8sato s8sato force-pushed the feat/1387 branch 2 times, most recently from 816098c to 0bd819a Compare November 30, 2021 12:24
appetrosyan
appetrosyan previously approved these changes Nov 30, 2021
This reverts commit b228b41.

Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
After your peer unregisters and disconnects another peer,
your network will hear reconnection requests from the peer.
All you can know at first is the address whose port number is arbitrary.
So remember the unregistered peer by the part other than the port number
and refuse reconnection from there

Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
Signed-off-by: s8sato <49983831+s8sato@users.noreply.github.com>
@s8sato s8sato changed the title Add the number of connected peers to Status [Feat] Add the number of connected peers to Status Nov 30, 2021
@appetrosyan appetrosyan merged commit 60cbd05 into hyperledger:iroha2-dev Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries iroha2-dev The re-implementation of a BFT hyperledger in RUST metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Endpoint for smoke testing network status
4 participants