[Backport] #2936 to release-2.2

[kopaygorodsky]

Backport of #2936

[yacovm]

I don't understand why you need to cherry pick that.

Doesn't the below git patch solve the problem?

diff --git a/orderer/common/multichannel/registrar.go b/orderer/common/multichannel/registrar.go
index 0b96e922f..48c37977d 100644
--- a/orderer/common/multichannel/registrar.go
+++ b/orderer/common/multichannel/registrar.go
@@ -11,6 +11,7 @@ package multichannel
 
 import (
        "fmt"
+       "github.com/hyperledger/fabric/orderer/consensus/etcdraft"
        "sync"
 
        cb "github.com/hyperledger/fabric-protos-go/common"
@@ -348,7 +349,10 @@ func (r *Registrar) CreateChain(chainName string) {
        if chain != nil {
                logger.Infof("A chain of type %T for channel %s already exists. "+
                        "Halting it.", chain.Chain, chainName)
+               r.lock.Lock()
                chain.Halt()
+               delete(r.chains, chainName)
+               r.lock.Unlock()
        }
        r.newChain(configTx(lf))
 }
@@ -357,6 +361,21 @@ func (r *Registrar) newChain(configtx *cb.Envelope) {
        r.lock.Lock()
        defer r.lock.Unlock()
 
+
+       channelName, err := channelNameFromConfigTx(configtx)
+       if err != nil {
+               logger.Warnf("Failed extracting channel name: %v", err)
+               return
+       }
+
+       // fixes https://github.com/hyperledger/fabric/issues/2931
+       if existingChain, exists := r.chains[channelName]; exists {
+               if _, isRaftChain := existingChain.Chain.(*etcdraft.Chain); isRaftChain {
+                       logger.Infof("Channel %s already created, skipping its creation", channelName)
+                       return
+               }
+       }
+
        ledgerResources, err := r.newLedgerResources(configtx)
        if err != nil {
                logger.Panicf("Error creating ledger resources: %s", err)
@@ -378,6 +397,25 @@ func (r *Registrar) newChain(configtx *cb.Envelope) {
        cs.start()
 }
 
+
+func channelNameFromConfigTx(configtx *cb.Envelope) (string, error) {
+       payload, err := protoutil.UnmarshalPayload(configtx.Payload)
+       if err != nil {
+               return "", errors.WithMessage(err, "error umarshaling envelope to payload")
+       }
+
+       if payload.Header == nil {
+               return "", errors.New("missing channel header")
+       }
+
+       chdr, err := protoutil.UnmarshalChannelHeader(payload.Header.ChannelHeader)
+       if err != nil {
+               return "", errors.WithMessage(err, "error unmarshalling channel header")
+       }
+
+       return chdr.ChannelId, nil
+}
+
 // ChannelsCount returns the count of the current total number of channels.
 func (r *Registrar) ChannelsCount() int {
        r.lock.RLock()

[kopaygorodsky]

@yacovm there is a dep cycle between multichannel and consensus/etcdraft. Doing _, isRaftChain := existingChain.Chain.(*etcdraft.Chain) in registrar is not possible at the moment. #1862 eliminates this cycle.

[yacovm]

All right, try the solution below.

I would just prefer to make backports as small as possible :-)

diff --git a/orderer/common/multichannel/registrar.go b/orderer/common/multichannel/registrar.go
index 0b96e922f..5c5a50153 100644
--- a/orderer/common/multichannel/registrar.go
+++ b/orderer/common/multichannel/registrar.go
@@ -348,7 +348,10 @@ func (r *Registrar) CreateChain(chainName string) {
        if chain != nil {
                logger.Infof("A chain of type %T for channel %s already exists. "+
                        "Halting it.", chain.Chain, chainName)
+               r.lock.Lock()
                chain.Halt()
+               delete(r.chains, chainName)
+               r.lock.Unlock()
        }
        r.newChain(configTx(lf))
 }
@@ -357,6 +360,21 @@ func (r *Registrar) newChain(configtx *cb.Envelope) {
        r.lock.Lock()
        defer r.lock.Unlock()
 
+
+       channelName, err := channelNameFromConfigTx(configtx)
+       if err != nil {
+               logger.Warnf("Failed extracting channel name: %v", err)
+               return
+       }
+
+       // fixes https://github.com/hyperledger/fabric/issues/2931
+       if existingChain, exists := r.chains[channelName]; exists {
+               if raftChain, isRaftChain := existingChain.Chain.(RaftChain); isRaftChain && raftChain.IsRaft() {
+                       logger.Infof("Channel %s already created, skipping its creation", channelName)
+                       return
+               }
+       }
+
        ledgerResources, err := r.newLedgerResources(configtx)
        if err != nil {
                logger.Panicf("Error creating ledger resources: %s", err)
@@ -378,6 +396,25 @@ func (r *Registrar) newChain(configtx *cb.Envelope) {
        cs.start()
 }
 
+
+func channelNameFromConfigTx(configtx *cb.Envelope) (string, error) {
+       payload, err := protoutil.UnmarshalPayload(configtx.Payload)
+       if err != nil {
+               return "", errors.WithMessage(err, "error umarshaling envelope to payload")
+       }
+
+       if payload.Header == nil {
+               return "", errors.New("missing channel header")
+       }
+
+       chdr, err := protoutil.UnmarshalChannelHeader(payload.Header.ChannelHeader)
+       if err != nil {
+               return "", errors.WithMessage(err, "error unmarshalling channel header")
+       }
+
+       return chdr.ChannelId, nil
+}
+
 // ChannelsCount returns the count of the current total number of channels.
 func (r *Registrar) ChannelsCount() int {
        r.lock.RLock()
@@ -494,3 +531,7 @@ func (r *Registrar) RemoveChannel(channelID string, removeStorage bool) error {
        //TODO
        return errors.New("Not implemented yet")
 }
+
+type RaftChain interface {
+       IsRaft() bool
+}
\ No newline at end of file
diff --git a/orderer/consensus/etcdraft/chain.go b/orderer/consensus/etcdraft/chain.go
index fbcda4f29..6d74007fb 100644
--- a/orderer/consensus/etcdraft/chain.go
+++ b/orderer/consensus/etcdraft/chain.go
@@ -1433,3 +1433,7 @@ func (c *Chain) triggerCatchup(sn *raftpb.Snapshot) {
        case <-c.doneC:
        }
 }
+
+func (c *Chain) IsRaft() bool {
+       return true
+}

[kopaygorodsky]

@yacovm oh, ok, I didn't know I could do a different implementation, from my point of view it would be hard to track and compare with the master branch.
I'll fix it in a bit.

[yacovm]

@yacovm oh, ok, I didn't know I could do a different implementation, from my point of view it would be hard to track and compare with the master branch. I'll fix it in a bit.

We never look back, only forward. Unless there is a bug and then we look back.
But, since FAB-2931 only introduced new lines and never removed lines, then we will never need to backport another fix that touches any of the lines of FAB-2931 unless there is a bug in the bug fix itself!

[kopaygorodsky]

@yacovm I think it's ready to be reviewed.