-
Notifications
You must be signed in to change notification settings - Fork 291
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
multi: Add started and done reorg notifications. #1495
Conversation
Sample simnet reorg output:
|
blockchain/chain.go
Outdated
b.sendNotification(NTChainReorgStarted, &ChainReorgStarted{}) | ||
b.chainLock.Lock() | ||
|
||
reorgDone := func() { |
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.
Can just defer this:
defer func() {
// Send a notification announcing the end of the chain reorganization.
b.chainLock.Unlock()
b.sendNotification(NTChainReorgDone, nil)
b.chainLock.Lock()
}()
blockchain/chain.go
Outdated
@@ -1353,6 +1365,7 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error | |||
parent = detachBlocks[i+1] | |||
} | |||
if n.parent.hash != *parent.Hash() { | |||
reorgDone() |
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.
No point in doing this before panic, but can be removed anyway with suggested defer above.
blockmanager.go
Outdated
@@ -1929,6 +1929,14 @@ func (b *blockManager) handleNotifyMsg(notification *blockchain.Notification) { | |||
r.ntfnMgr.NotifyBlockDisconnected(block) | |||
} | |||
|
|||
// Chain reorganization has commenced. | |||
case blockchain.NTChainReorgStarted: | |||
bmgrLog.Infof("Chain reorganization started.") |
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.
No reason to log this.
blockmanager.go
Outdated
|
||
// Chain reorganization has concluded. | ||
case blockchain.NTChainReorgDone: | ||
bmgrLog.Infof("Chain reorganization ended.") |
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.
Nor this.
blockchain/notifications.go
Outdated
@@ -119,6 +127,12 @@ type ReorganizationNtfnsData struct { | |||
NewHeight int64 | |||
} | |||
|
|||
// ChainReorgStarted signals the commencement of a chain reorganization. | |||
type ChainReorgStarted 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.
I don't see a reason for this type since it's empty anyway. Just use nil
in the ntfn.
blockchain/notifications.go
Outdated
@@ -137,6 +151,8 @@ type TicketNotificationsData struct { | |||
// - NTBlockAccepted: *BlockAcceptedNtfnsData | |||
// - NTBlockConnected: []*dcrutil.Block of len 2 | |||
// - NTBlockDisconnected: []*dcrutil.Block of len 2 | |||
// - NTChainReorgStarted: *ChainReorgStarted |
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.
nil
blockchain/notifications.go
Outdated
@@ -137,6 +151,8 @@ type TicketNotificationsData struct { | |||
// - NTBlockAccepted: *BlockAcceptedNtfnsData | |||
// - NTBlockConnected: []*dcrutil.Block of len 2 | |||
// - NTBlockDisconnected: []*dcrutil.Block of len 2 | |||
// - NTChainReorgStarted: *ChainReorgStarted | |||
// - NTChainReorgDone: *ChainReorgDone |
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.
nil
blockchain/chain.go
Outdated
@@ -1331,6 +1331,18 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error | |||
b.sendNotification(NTReorganization, reorgData) | |||
b.chainLock.Lock() | |||
|
|||
// Send a notification announcing the start of the chain reorganization. | |||
b.chainLock.Unlock() | |||
b.sendNotification(NTChainReorgStarted, &ChainReorgStarted{}) |
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.
See other comments. No need to create an empty heap object that has to be GC'd. Just use nil
.
b.sendNotification(NTChainReorgStarted, nil)
blockchain/notifications.go
Outdated
type ChainReorgStarted struct{} | ||
|
||
// ChainReorgDone signals the conclusion of a chain reorganization. | ||
type ChainReorgDone 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.
Same here.
This adds NTChainReorgStarted and NTChainReorgDone notifications to the blockmanager.
This adds
NTChainReorgStarted
andNTChainReorgDone
notifications to the blockmanager.