[FAB-17774] support orderer restart without genesis block #1197
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yacovm
reviewed
Apr 29, 2020
yacovm
reviewed
Apr 29, 2020
yacovm
reviewed
Apr 29, 2020
yacovm
reviewed
Apr 29, 2020
| etcdConsenter := initializeEtcdraftConsenter(consenters, conf, lf, clusterDialer, bootstrapBlock, ri, srvConf, srv, registrar, metricsProvider, bccsp) | ||
| icr = etcdConsenter.InactiveChainRegistry | ||
| if conf.General.BootstrapMethod == "file" || conf.General.BootstrapMethod == "none" { | ||
| if bootstrapBlock != nil && isClusterType(bootstrapBlock, bccsp) { |
There was a problem hiding this comment.
can the bootstrapBlock be nil here? We don't have the check on the left side, so how can it be nil? Wouldn't we have crashed because we didn't have this check in the past?
And let's say bootstrap block is nil, doesn't that mean that you don't initialize the etcdraft? Isn't that a problem?
There was a problem hiding this comment.
bootstrapBlock should not be nil here, however, in test TestInitializeMultichannelRegistrar/"registrar without a system channel", it directly calls initializeMultichannelRegistrar, and passed in bootstrapBlock as nil, and BootstrapMethod as "none". In this case, the test will pass the old code as it will only check cluster type if BootstrapMethod is "file", but not the new code.
There was a problem hiding this comment.
So why not not pass a nil bootstrap block then? we shouldn't change the production code just to fit tests...
There was a problem hiding this comment.
It's definitely good point, we should never change production code to just fit tests.
However, currently orderer supports an action to startup without system channel defined (FAB-15709), which sets BootstrapMethod to "none" and bootstrapBlock to be nil.
And in this PR, we are supporting restarting orderer without providing genesisblock, which was being requested to set BootstrapMethod to "none", and obtain bootstrapBlock from system channel (FAB-17774). Hence, the nil check here is to distinguish the two cases.
We can also adding a new bootstrapMethod phrase here, instead of using "none" for both use cases. I'm opening to any suggestions.
yacovm
reviewed
Apr 29, 2020
| gt.Eventually(ordererProcess.Err, time.Minute).Should(gbytes.Say("Beginning to serve requests")) | ||
| gt.Eventually(ordererProcess.Err, time.Minute).Should(gbytes.Say("becomeLeader")) | ||
|
|
||
| // Restart orderer with ORDERER_GENERAL_BOOTSTRAPMETHOD = none |
There was a problem hiding this comment.
I am not sure this is enough... shouldn't we also delete the file?
* adding support for orderer restart without genesis block. Signed-off-by: Chongxin Luo <Chongxin.Luo@ibm.com>
yacovm
approved these changes
May 1, 2020
xhens
added a commit
to xhens/fabric
that referenced
this pull request
May 8, 2020
* [FAB-17819] Discovery returns user friendly errors Currently, the discovery service filters out peers that don't have the chaincode installed early on in the computation, and as a result - the service cannot distinguish from a case where there are not enough alive peers to satisfy the endorsement policy, or that there are enough peers but the chaincode is not installed on enough of them. This change set defers the chaincode filtering to the end of the computation, so the layouts and peer group mapping is creating without taking into account if the peers have the chaincode installed on them, and if there is no layout that can be satisfied without taking into account the chaincodes - the error that is returned now is "no peer combination can satisfy the endorsement policy", instead of "cannot satisfy any principal combination". Afterwards, the layouts are being inspected once again, and then the layouts that cannot be satisfied are filtered out, when the error returned when no layout can be satisfied is now: "required chaincodes are not installed on sufficient peers". Change-Id: I74eb29b30aec1a87842d220414c73872cdbc8304 Signed-off-by: yacovm <yacovm@il.ibm.com> * Fix docker network leak from RAFT integration test (hyperledger#1203) Signed-off-by: Matthew Sykes <matthew.sykes@gmail.com> * [FAB-17786] Update upgrade_dbs peer command to drop state couchdb (hyperledger#1187) The existing upgrade_dbs command does not automatically drop state couchdbs and therefore a separate step is required to drop couchdbs, This PR updates the command to automatically drop state couchdbs. In addition, it checks upgrade eligibility before upgrade so that it will not drop databases if it is already the expected format. Signed-off-by: Wenjian Qiao <wenjianq@gmail.com> * [FAB-17774] support orderer restart without genesis block (hyperledger#1197) * adding support for orderer restart without genesis block. Signed-off-by: Chongxin Luo <Chongxin.Luo@ibm.com> * Remove interface for block storage - Removed the interface for better code navigation, as There is only a single implementation for block storage - Merged the remaining code in the package blkstorgae with the implementation in the package fsblkstorage and used the name blkstorgae for the final package - Moved the internal single proto message with in same package Signed-off-by: manish <manish.sethi@gmail.com> * [LEDGER] Move UpdatesBytesBuilder to txmgr pkg (hyperledger#1209) - mv updateBatch bytes constructor to txmgr pkg Currently, we create the bytes representation of updateBatch to compute the hash of updateBatch which would be included in the block along with the validation results. The utility for converting a updateBatch to a deterministic bytes is present in the privacyenabledstate pkg. However, this utility is used only by the txmgr and hence, we move the utility function to txmgr pkg from privacyenabledstate pkg. - rename proto messages In the updateBatch proto, we have defined proto messages such as KVWriteProto and KVWriteBatchProto. As we shouldn't append the keyword proto to messages, we rename KVWriteProto to KVWrite and KVWriteBatchProto to Updates. - rename function names The function name such as buildForKeys(), buildForColls() are not very explicit in what they do. When the buildForColls() calls buildForKeys(), it adds even more confusion as the first-level function, i.e., deterministicBytesForPubAndHashUpdates() is also calling buildForKeys(). Hence, we have used the following function names instead: (1) genKVsFromNsUpdates() (2) genKVsFromCollsUpdates() (3) genKVs() FAB-17830 Signed-off-by: senthil <cendhu@gmail.com> * Update grpc-go to v1.29.1 (hyperledger#1213) Signed-off-by: Gari Singh <gari.r.singh@gmail.com> * [FAB-17831] Use generic constant/var names in dataformat.go (hyperledger#1212) Currently Version1x and Version20 are defined in dataformat.go. They should be renamed to more generic names such as PreviousFormat and CurrentFormat. The format values will change only when the data format is changed in ledger. If a Fabric version does not introduce a new data format, CurrentFormat will remain the same as the latest format prior to the Fabric version. Signed-off-by: Wenjian Qiao <wenjianq@gmail.com> * [LEDGER] rm interface from pvtdatastorage pkg (hyperledger#1217) rm pvtdatastorage interfaces As we have only one implementation of the pvtdatastore and not planning to any a new one, we can safely remove the interface. This also helps in code navigation and to avoid type casting such as s.(*store) in the test. FAB-17843 Signed-off-by: senthil <cendhu@gmail.com> * FAB-15710 Ch.Part.API: orderer config & hook into http server (hyperledger#1218) - Expose a configuration object for channel participation at the top level (like Metrics). Even though the channel participation API shares the same endpoint and TLS config with operations, placing it at the top level will allow us to separate these APIs to different endpoints in the future without changing the config structure. - Extend operations.System to be able to register a handler for additional APIs. - Implement a skeleton handler for the channel participation API. - Register the skeleton handler to the http server in operations.System during the server boot sequence. Signed-off-by: Yoav Tock <tock@il.ibm.com> Change-Id: I5cf15dffa29985cba60e5aaf31d189e755a3a1ef * Update the vagrant dev environment - Remove GOPATH in favor of modules - Move to Ubuntu 20.04 - Remove docker-compose as it's unnecessary for build and test Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com> * Add function in blockstore to export TxIDs FAB-17837 Signed-off-by: manish <manish.sethi@gmail.com> * reduce #arguments in a few kvledger methods (hyperledger#1210) As the number of arguments passed to newKVLedger(), initTxMgr() is quite high, we reduce it by introducing initializer struct. FAB-17683 Signed-off-by: senthil <cendhu@gmail.com> Co-authored-by: yacovm <yacovm@il.ibm.com> Co-authored-by: Matthew Sykes <sykesmat@us.ibm.com> Co-authored-by: wen <wenjianq@gmail.com> Co-authored-by: Dereck <Chongxin.Luo@ibm.com> Co-authored-by: manish <manish.sethi@gmail.com> Co-authored-by: Senthil Nathan N <cendhu@users.noreply.github.com> Co-authored-by: Gari Singh <gari.r.singh@gmail.com> Co-authored-by: Yoav Tock <tock@il.ibm.com>
xhens
added a commit
to xhens/fabric
that referenced
this pull request
May 10, 2020
* [FAB-17819] Discovery returns user friendly errors Currently, the discovery service filters out peers that don't have the chaincode installed early on in the computation, and as a result - the service cannot distinguish from a case where there are not enough alive peers to satisfy the endorsement policy, or that there are enough peers but the chaincode is not installed on enough of them. This change set defers the chaincode filtering to the end of the computation, so the layouts and peer group mapping is creating without taking into account if the peers have the chaincode installed on them, and if there is no layout that can be satisfied without taking into account the chaincodes - the error that is returned now is "no peer combination can satisfy the endorsement policy", instead of "cannot satisfy any principal combination". Afterwards, the layouts are being inspected once again, and then the layouts that cannot be satisfied are filtered out, when the error returned when no layout can be satisfied is now: "required chaincodes are not installed on sufficient peers". Change-Id: I74eb29b30aec1a87842d220414c73872cdbc8304 Signed-off-by: yacovm <yacovm@il.ibm.com> * Fix docker network leak from RAFT integration test (hyperledger#1203) Signed-off-by: Matthew Sykes <matthew.sykes@gmail.com> * [FAB-17786] Update upgrade_dbs peer command to drop state couchdb (hyperledger#1187) The existing upgrade_dbs command does not automatically drop state couchdbs and therefore a separate step is required to drop couchdbs, This PR updates the command to automatically drop state couchdbs. In addition, it checks upgrade eligibility before upgrade so that it will not drop databases if it is already the expected format. Signed-off-by: Wenjian Qiao <wenjianq@gmail.com> * [FAB-17774] support orderer restart without genesis block (hyperledger#1197) * adding support for orderer restart without genesis block. Signed-off-by: Chongxin Luo <Chongxin.Luo@ibm.com> * Remove interface for block storage - Removed the interface for better code navigation, as There is only a single implementation for block storage - Merged the remaining code in the package blkstorgae with the implementation in the package fsblkstorage and used the name blkstorgae for the final package - Moved the internal single proto message with in same package Signed-off-by: manish <manish.sethi@gmail.com> * [LEDGER] Move UpdatesBytesBuilder to txmgr pkg (hyperledger#1209) - mv updateBatch bytes constructor to txmgr pkg Currently, we create the bytes representation of updateBatch to compute the hash of updateBatch which would be included in the block along with the validation results. The utility for converting a updateBatch to a deterministic bytes is present in the privacyenabledstate pkg. However, this utility is used only by the txmgr and hence, we move the utility function to txmgr pkg from privacyenabledstate pkg. - rename proto messages In the updateBatch proto, we have defined proto messages such as KVWriteProto and KVWriteBatchProto. As we shouldn't append the keyword proto to messages, we rename KVWriteProto to KVWrite and KVWriteBatchProto to Updates. - rename function names The function name such as buildForKeys(), buildForColls() are not very explicit in what they do. When the buildForColls() calls buildForKeys(), it adds even more confusion as the first-level function, i.e., deterministicBytesForPubAndHashUpdates() is also calling buildForKeys(). Hence, we have used the following function names instead: (1) genKVsFromNsUpdates() (2) genKVsFromCollsUpdates() (3) genKVs() FAB-17830 Signed-off-by: senthil <cendhu@gmail.com> * Update grpc-go to v1.29.1 (hyperledger#1213) Signed-off-by: Gari Singh <gari.r.singh@gmail.com> * [FAB-17831] Use generic constant/var names in dataformat.go (hyperledger#1212) Currently Version1x and Version20 are defined in dataformat.go. They should be renamed to more generic names such as PreviousFormat and CurrentFormat. The format values will change only when the data format is changed in ledger. If a Fabric version does not introduce a new data format, CurrentFormat will remain the same as the latest format prior to the Fabric version. Signed-off-by: Wenjian Qiao <wenjianq@gmail.com> * [LEDGER] rm interface from pvtdatastorage pkg (hyperledger#1217) rm pvtdatastorage interfaces As we have only one implementation of the pvtdatastore and not planning to any a new one, we can safely remove the interface. This also helps in code navigation and to avoid type casting such as s.(*store) in the test. FAB-17843 Signed-off-by: senthil <cendhu@gmail.com> * FAB-15710 Ch.Part.API: orderer config & hook into http server (hyperledger#1218) - Expose a configuration object for channel participation at the top level (like Metrics). Even though the channel participation API shares the same endpoint and TLS config with operations, placing it at the top level will allow us to separate these APIs to different endpoints in the future without changing the config structure. - Extend operations.System to be able to register a handler for additional APIs. - Implement a skeleton handler for the channel participation API. - Register the skeleton handler to the http server in operations.System during the server boot sequence. Signed-off-by: Yoav Tock <tock@il.ibm.com> Change-Id: I5cf15dffa29985cba60e5aaf31d189e755a3a1ef * Update the vagrant dev environment - Remove GOPATH in favor of modules - Move to Ubuntu 20.04 - Remove docker-compose as it's unnecessary for build and test Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com> * Add function in blockstore to export TxIDs FAB-17837 Signed-off-by: manish <manish.sethi@gmail.com> * reduce #arguments in a few kvledger methods (hyperledger#1210) As the number of arguments passed to newKVLedger(), initTxMgr() is quite high, we reduce it by introducing initializer struct. FAB-17683 Signed-off-by: senthil <cendhu@gmail.com> * Clarify the deliver access denied message (hyperledger#1224) There are two scenarios where a deliver client could receive a 'FORBIDDEN' result when requesting blocks. Either the client was not authorized to connect to the channel initially, or, the client's access was revoked after a successful connection by some later configuration block. In both cases, we log an identical error message that "Client authorization revoked" when in fact, for the first case, the client may never have had access, so claiming it was revoked is misleading. Signed-off-by: Jason Yellick <jyellick@us.ibm.com> * Validate TLS certs during raft consenter addition (hyperledger#1223) * raft membership_test.go cleanup - For negative tests, test that expected error message is returned. - For positive tests, switch the order so expected/actual match the expected usage of "testify/require" package. Signed-off-by: Will Lahti <wtlahti@us.ibm.com> * Validate TLS certs during raft consenter addition FAB-17733 #done Signed-off-by: Will Lahti <wtlahti@us.ibm.com> * [FAB-17640] Remove pkg/configtx and import as hyperledger/fabric-config Signed-off-by: Danny Cao <dcao@us.ibm.com> * Update CONTRIBUTING guide Removed reference to Gerrit Signed-off-by: Ry Jones <ry@linux.com> Co-authored-by: yacovm <yacovm@il.ibm.com> Co-authored-by: Matthew Sykes <sykesmat@us.ibm.com> Co-authored-by: wen <wenjianq@gmail.com> Co-authored-by: Dereck <Chongxin.Luo@ibm.com> Co-authored-by: manish <manish.sethi@gmail.com> Co-authored-by: Senthil Nathan N <cendhu@users.noreply.github.com> Co-authored-by: Gari Singh <gari.r.singh@gmail.com> Co-authored-by: Yoav Tock <tock@il.ibm.com> Co-authored-by: Jason Yellick <jyellick@us.ibm.com> Co-authored-by: Will Lahti <wtlahti@us.ibm.com> Co-authored-by: Danny Cao <dcao@us.ibm.com> Co-authored-by: Ry Jones <ry@linux.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Chongxin Luo Chongxin.Luo@ibm.com
Type of change
Description
Related issues
FAB-17774