Skip to content

Commit

Permalink
[FAB-3520] Eventer doesn't trigger subsequent updates
Browse files Browse the repository at this point in the history
The eventer applies the config on an object that is returned from
an invocation of config.Organizations(), which returns the same
reference of map in each config update.

The config.Organizations() is a map from org name to config.ApplicationOrg
which has the anchor peers.

Due to the fact that the references are the same,
the ce.lastConfig.orgMap is the same as the new orgMap of a new config update,
and this makes the logic think that there has not been any update of
anchor peers at every real update (of anchor peers).

I fixed the bug by cloning the map, and also fixed the test
in eventer_test.go(line 98) to fail when the code of the eventer.go
is before the fix.

Change-Id: Iede772975216a88d00badfc9d7092f92580737a8
Signed-off-by: Yacov Manevich <yacovm@il.ibm.com>
  • Loading branch information
yacovm committed Apr 30, 2017
1 parent c69a25e commit 08df4e3
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 49 deletions.
36 changes: 33 additions & 3 deletions gossip/service/eventer.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func newConfigEventer(receiver configEventReceiver) *configEventer {
// Note, that a changing sequence number is ignored as changing configuration
func (ce *configEventer) ProcessConfigUpdate(config Config) {
logger.Debugf("Processing new config for channel %s", config.ChainID())

if ce.lastConfig != nil && reflect.DeepEqual(ce.lastConfig.orgMap, config.Organizations()) {
orgMap := cloneOrgConfig(config.Organizations())
if ce.lastConfig != nil && reflect.DeepEqual(ce.lastConfig.orgMap, orgMap) {
logger.Debugf("Ignoring new config for channel %s because it contained no anchor peer updates", config.ChainID())
return
}
Expand All @@ -80,11 +80,41 @@ func (ce *configEventer) ProcessConfigUpdate(config Config) {
}

newConfig := &configStore{
orgMap: config.Organizations(),
orgMap: orgMap,
anchorPeers: newAnchorPeers,
}
ce.lastConfig = newConfig

logger.Debugf("Calling out because config was updated for channel %s", config.ChainID())
ce.receiver.configUpdated(config)
}

func cloneOrgConfig(src map[string]config.ApplicationOrg) map[string]config.ApplicationOrg {
clone := make(map[string]config.ApplicationOrg)
for k, v := range src {
clone[k] = &appGrp{
name: v.Name(),
mspID: v.MSPID(),
anchorPeers: v.AnchorPeers(),
}
}
return clone
}

type appGrp struct {
name string
mspID string
anchorPeers []*peer.AnchorPeer
}

func (ag *appGrp) Name() string {
return ag.name
}

func (ag *appGrp) MSPID() string {
return ag.mspID
}

func (ag *appGrp) AnchorPeers() []*peer.AnchorPeer {
return ag.anchorPeers
}
65 changes: 20 additions & 45 deletions gossip/service/eventer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,10 @@ import (

const testChainID = "foo"

type applicationOrgs []*peer.AnchorPeer

func init() {
util.SetupTestLogging()
}

func (ao applicationOrgs) AnchorPeers() []*peer.AnchorPeer {
return ao
}

func (ao applicationOrgs) MSPID() string {
return "ORG1"
}

func (ao applicationOrgs) Name() string {
panic("Unimplimented")
}

type mockReceiver struct {
orgs map[string]config.ApplicationOrg
sequence uint64
Expand Down Expand Up @@ -76,11 +62,9 @@ func TestInitialUpdate(t *testing.T) {
mc := &mockConfig{
sequence: 7,
orgs: map[string]config.ApplicationOrg{
testOrgID: applicationOrgs([]*peer.AnchorPeer{
&peer.AnchorPeer{
Port: 9,
},
}),
testOrgID: &appGrp{
anchorPeers: []*peer.AnchorPeer{{Port: 9}},
},
},
}

Expand All @@ -95,15 +79,14 @@ func TestInitialUpdate(t *testing.T) {
}

func TestSecondUpdate(t *testing.T) {
appGrps := map[string]config.ApplicationOrg{
testOrgID: &appGrp{
anchorPeers: []*peer.AnchorPeer{{Port: 9}},
},
}
mc := &mockConfig{
sequence: 7,
orgs: map[string]config.ApplicationOrg{
testOrgID: applicationOrgs([]*peer.AnchorPeer{
&peer.AnchorPeer{
Port: 9,
},
}),
},
orgs: appGrps,
}

mr := &mockReceiver{}
Expand All @@ -112,30 +95,24 @@ func TestSecondUpdate(t *testing.T) {
ce.ProcessConfigUpdate(mc)

mc.sequence = 8
mc.orgs = map[string]config.ApplicationOrg{
testOrgID: applicationOrgs([]*peer.AnchorPeer{
&peer.AnchorPeer{
Port: 10,
},
}),
appGrps[testOrgID] = &appGrp{
anchorPeers: []*peer.AnchorPeer{{Port: 10}},
}

ce.ProcessConfigUpdate(mc)

if !reflect.DeepEqual(mc, (*mockConfig)(mr)) {
t.Fatalf("Should have updated config on initial update but did not")
t.Fatal("Should have updated config on initial update but did not")
}
}

func TestSecondSameUpdate(t *testing.T) {
mc := &mockConfig{
sequence: 7,
orgs: map[string]config.ApplicationOrg{
testOrgID: applicationOrgs([]*peer.AnchorPeer{
&peer.AnchorPeer{
Port: 9,
},
}),
testOrgID: &appGrp{
anchorPeers: []*peer.AnchorPeer{{Port: 9}},
},
},
}

Expand All @@ -148,23 +125,21 @@ func TestSecondSameUpdate(t *testing.T) {
ce.ProcessConfigUpdate(mc)

if mr.sequence != 0 {
t.Errorf("Should not have updated sequence when reprocessing same config")
t.Error("Should not have updated sequence when reprocessing same config")
}

if mr.orgs != nil {
t.Errorf("Should not have updated anchor peers when reprocessing same config")
t.Error("Should not have updated anchor peers when reprocessing same config")
}
}

func TestUpdatedSeqOnly(t *testing.T) {
mc := &mockConfig{
sequence: 7,
orgs: map[string]config.ApplicationOrg{
testOrgID: applicationOrgs([]*peer.AnchorPeer{
&peer.AnchorPeer{
Port: 9,
},
}),
testOrgID: &appGrp{
anchorPeers: []*peer.AnchorPeer{{Port: 9}},
},
},
}

Expand Down
5 changes: 4 additions & 1 deletion gossip/service/gossip_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,10 @@ func TestChannelConfig(t *testing.T) {
mc := &mockConfig{
sequence: 1,
orgs: map[string]config.ApplicationOrg{
testOrgID: applicationOrgs([]*peer.AnchorPeer{}),
string(orgInChannelA): &appGrp{
mspID: string(orgInChannelA),
anchorPeers: []*peer.AnchorPeer{},
},
},
}
gService.JoinChan(jcm, gossipCommon.ChainID("A"))
Expand Down

0 comments on commit 08df4e3

Please sign in to comment.