-
Notifications
You must be signed in to change notification settings - Fork 126
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
refactor: model finalised notifier channel after imported notifier channel #1816
Conversation
Codecov Report
@@ Coverage Diff @@
## development #1816 +/- ##
===============================================
- Coverage 59.66% 59.62% -0.04%
===============================================
Files 189 188 -1
Lines 19673 19621 -52
===============================================
- Hits 11738 11700 -38
+ Misses 5960 5952 -8
+ Partials 1975 1969 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -218,7 +217,7 @@ func TestWSConn_HandleComm(t *testing.T) { | |||
require.NoError(t, err) | |||
require.Equal(t, `{"jsonrpc":"2.0","method":"author_extrinsicUpdate","params":{"result":"ready","subscription":8}}`+"\n", string(msg)) | |||
|
|||
var fCh chan<- *types.FinalisationInfo | |||
//var fCh chan *types.FinalisationInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good :D
|
||
bs.finalisedLock.RUnlock() | ||
ch := make(chan *types.FinalisationInfo, DEFAULT_BUFFER_SIZE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEFAULT_BUFFER_SIZE
make this camelcase.
@@ -74,7 +73,7 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e | |||
baseState: NewBaseState(db), | |||
db: chaindb.NewTable(db, blockPrefix), | |||
imported: make(map[chan *types.Block]struct{}), | |||
finalised: make(map[byte]chan<- *types.FinalisationInfo), | |||
finalised: make(map[chan *types.FinalisationInfo]struct{}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I didn't know we could create a map with channels as key.
for _, ch := range bs.finalised { | ||
go func(ch chan<- *types.FinalisationInfo) { | ||
for ch := range bs.finalised { | ||
go func(ch chan *types.FinalisationInfo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go func(ch chan *types.FinalisationInfo) { | |
go func(ch chan<- *types.FinalisationInfo) { |
if err != nil { | ||
logger.Error("failed to unregister finalised channel", "error", err) | ||
} | ||
delete(bs.finalised, ch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explicitly close the channel
close(ch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// FreeImportedBlockNotifierChannel to free imported block notifier channel
func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) {
bs.importedLock.Lock()
defer bs.importedLock.Unlock()
delete(bs.imported, ch)
close(ch)
}
// FreeFinalisedNotifierChannel to free finalized notifier channel
func (bs *BlockState) FreeFinalisedNotifierChannel(ch chan *types.FinalisationInfo) {
bs.finalisedLock.Lock()
defer bs.finalisedLock.Unlock()
delete(bs.finalised, ch)
close(ch)
}
🎉 This PR is included in version 0.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Changes
Tests
Issues
Primary Reviewer