Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

BlockAllocate rpc returns all peers when replication factor -1 but should return healthy ones only #543

Closed
10 of 14 tasks
hsanjuan opened this issue Sep 21, 2018 · 6 comments
Closed
10 of 14 tasks
Assignees
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up

Comments

@hsanjuan
Copy link
Collaborator

hsanjuan commented Sep 21, 2018

Pre-check

  • This is not a IPFS Cluster website content issue (file those here)
  • I read the troubleshooting section of the website and it did not help
  • I searched for similar issues in the repo without luck
  • All my peers are running the same cluster version
  • All my peers are configured using the same cluster secret

Basic information

  • Version information (mark as appropiate):
    • Master
    • Release candidate for next version
    • Latest stable version
    • An older version I should not be using
  • Type (mark as appropiate):
    • Bug
    • Feature request
    • Enhancement

Description

The bug is here: https://github.com/ipfs/ipfs-cluster/blob/master/rpc_api.go#L205

When the adder allocates and replication factor is -1, we return all cluster peers. This ends up having the adder try to put blocks to peers which are down. We should use LatestMetrics instead from the monitor, with the "ping" metric type. And return the peerset extracted from those metrics (which are valid cluster peers).

@hsanjuan hsanjuan added kind/bug A bug in existing code (including security flaws) exp/novice Someone with a little familiarity can pick up status/ready Ready to be worked P1 High: Likely tackled by core team if no one steps up labels Sep 21, 2018
@mike-ngu
Copy link

Hi guys!

Could you tell me please when are you planning to fix this bug and what I need to do to apply this updates?

If I not mistaken current version of IPFS is v0.4.17 and ipfs-cluster-service is version 0.5.0.
Does this mean that I have to wait until the next release to have this updates in my IPFS package?

@hsanjuan
Copy link
Collaborator Author

Hello. We will fix it for the next release. Hopefully this will come out in about two weeks.

@mike-ngu
Copy link

can't wait to test it

Thank you!

@hsanjuan hsanjuan self-assigned this Sep 26, 2018
@hsanjuan hsanjuan added status/in-progress In progress and removed status/ready Ready to be worked labels Sep 26, 2018
hsanjuan added a commit that referenced this issue Sep 26, 2018
License: MIT
Signed-off-by: Hector Sanjuan <code@hector.link>
hsanjuan added a commit that referenced this issue Sep 27, 2018
License: MIT
Signed-off-by: Hector Sanjuan <code@hector.link>
@ghost ghost removed the status/in-progress In progress label Sep 27, 2018
hsanjuan added a commit that referenced this issue Sep 27, 2018
Fix #543: Use only healthy peers when adding everywhere (+tests)
@hsanjuan hsanjuan mentioned this issue Oct 1, 2018
@hsanjuan
Copy link
Collaborator Author

hsanjuan commented Oct 1, 2018

Hello @mike-ngu , the a release candidate with this fix is available here: https://dist.ipfs.io/ipfs-cluster-service/

@mike-ngu
Copy link

mike-ngu commented Oct 2, 2018

Hello @hsanjuan,

Thank you for notification.

Just to clarify, to get last bug fixing I need to install ipfs-cluster-service package with version v0.6.0-rc1 ?

@hsanjuan
Copy link
Collaborator Author

hsanjuan commented Oct 2, 2018

yes, correct

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

No branches or pull requests

2 participants