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

bug fix 4509. Configuration parameters CA and address in peer.deliver… #4510

Open
wants to merge 1 commit into
base: release-2.4
Choose a base branch
from

Conversation

marianothiago
Copy link

Type of change

  • Bug fix

Description

Fabric reads the values stored in core.yaml in the peer.deliveryclient.addressOverrides section, but when connecting to orderers through the connect method in the blocksprovider.go file (internal/pkg/peer/blocksprovider/blocksprovider.go) it is using the RandomEndpoint function (endpoint, err := d.Orderers.RandomEndpoint()) located in the connection.go file (internal/pkg/peer/orderers/connection.go) to set up the endpoint that will make the connection. However the RandomEndpoint function does not use the peer.deliveryclient.addressOverrides information read by Fabric.

Additional details

Related issues

#4509

…yclient.addressOverrides area ignored when connecting to the orderers
@C0rWin
Copy link
Contributor

C0rWin commented Oct 31, 2023

Thanks for providing the fix. Would you also mind adding some unit tests to demonstrate the issue described and to make sure the fix indeed solves the problem? Moreover, to ensure we got it covered for potential future modifications?

@denyeart
Copy link
Contributor

Does the problem still occur in main branch? Typically fixes are made to main branch and then backported as desired to the release branches.

@marianothiago
Copy link
Author

Thanks for providing the fix. Would you also mind adding some unit tests to demonstrate the issue described and to make sure the fix indeed solves the problem? Moreover, to ensure we got it covered for potential future modifications?

OK. I will do this.

@marianothiago
Copy link
Author

Does the problem still occur in main branch? Typically fixes are made to main branch and then backported as desired to the release branches.

I tested branches 2.1, 2.2, 2.3, 2.4 and 2.5. I haven't tested it in main. I'll test it and let you know.

@tock-ibm
Copy link
Contributor

tock-ibm commented Nov 6, 2023

@marianothiago @C0rWin @denyeart

I don't think this is the right approach.
The override endpoints are first loaded from the core yaml when the peer starts. Then when ever

Update(globalAddrs []string, orgs map[string]OrdererOrg)

is called, after a config TX, the source slice is rebuilt (this is the way the current config is applied as well after the peer starts). The sources slice is rebuilt by comparing the overrides to the addresses that come from the config block - globalAddrs or per org orgs . If an orderer override has the same address as one of the endpoints in the globalAddrs or orgs, it will be used instead of that specific address. This is the place to look for a bug, if there is one, in my opinion. According to the current code, overrides are a mechanism to override the definition of an endpoint, not to add more endpoints, which is what you do here.

With respect to your use case: you describe a case where a peer falls behind the ordering cluster config, and cannot connect to it. While the overrides may give you a partial solution, I think the more "updated" way to approach this is by starting the peer from a snapshot taken from another peer that is up to date.

And finally, this object is about to change in V3, as a result of the work we do to implement a byzantine block puller.

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.

4 participants