-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
make journal pluggable; record deals, sealing, wdpost, mempool events #2455
Conversation
@@ -0,0 +1,131 @@ | |||
package journal |
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.
Most of this was copied as-is.
storage/wdpost_sched.go
Outdated
Proofs *WindowPoStEvt_Proofs `json:",omitempty"` | ||
Recoveries *WindowPoStEvt_Recoveries `json:",omitempty"` | ||
Faults *WindowPoStEvt_Faults `json:",omitempty"` |
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.
These could actually be splintered into separate event types.
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.
Probably worth doing, window post journal will likely be the one of the most looked at
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.
Makes sense, will do.
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.
Done.
markets/storageadapter/provider.go
Outdated
@@ -45,14 +46,25 @@ type ProviderNodeAdapter struct { | |||
|
|||
secb *sectorblocks.SectorBlocks | |||
ev *events.Events | |||
|
|||
jrnl journal.Journal | |||
evtTypes [4]journal.EventType |
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.
For performance reasons, an array of journal.EventType
s is what I settled on as the lowest overhead, nicely encapsulated technique to initialize and store EventTypes
during construction time, and index into them during runtime.
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.
Code which is called here triggers codepaths which take minutes/hours on real networks, I don't think we need to worry about making this super fast too much.
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.
Sorry for taking so long on this.
Looks pretty good, just a couple of comments
|
||
func (n *nilJournal) RegisterEventType(_, _ string) EventType { return EventType{} } | ||
|
||
func (n *nilJournal) RecordEvent(_ EventType, _ func() interface{}) {} |
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.
Should we scream loudly in logs when something calls this?
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 think so. This journal is designed for testing or for running stuff outside the lotus daemon (e.g. CLI tools). Things will call it all the time; this journal just no-ops. Maybe we can scream loudly when registering events, but I think it adds no value.
markets/storageadapter/provider.go
Outdated
@@ -45,14 +46,25 @@ type ProviderNodeAdapter struct { | |||
|
|||
secb *sectorblocks.SectorBlocks | |||
ev *events.Events | |||
|
|||
jrnl journal.Journal | |||
evtTypes [4]journal.EventType |
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.
Code which is called here triggers codepaths which take minutes/hours on real networks, I don't think we need to worry about making this super fast too much.
} | ||
|
||
// SealingStateEvt is a journal event that records a sector state transition. | ||
type SealingStateEvt 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.
Ideally this would live inside the sealing
package. We want to move sealing.New
construction to DI, but it's somewhat entangled with the Miner
struct here. We should try to not make this worse
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.
You mean the journalling itself, or just the struct? If it's the first, we could do that, but that means that the theoretically external sealing package would now depend on Lotus' journal interface, unless we re-declare it there. This would be a very simple refactor to do going forward, when compared to all the detangling that needs to happen anyway, so I wouldn't wanna stall on this right here right now.
storage/wdpost_sched.go
Outdated
Proofs *WindowPoStEvt_Proofs `json:",omitempty"` | ||
Recoveries *WindowPoStEvt_Recoveries `json:",omitempty"` | ||
Faults *WindowPoStEvt_Faults `json:",omitempty"` |
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.
Probably worth doing, window post journal will likely be the one of the most looked at
Note that I don’t have a miner, so I haven’t tested this to the degree I'd be satisfied. Please test with a miner of yours before merging! There may be hidden bugs that weren’t caught by tests. |
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.
lets doooo ittttt
This is freaking awesome <3 |
Please merge with a squash
Addresses part of #2441; the other part (in-memory journal implementation) is hosted in Project Oni: filecoin-project/oni#226.
What has been done here
New journal events
Journal framework
fx
to inject it where needed.DisabledEvents
fx key.MaybeRecordEvent
that lazily creates the event only if the EventType is enabled.Example journal events
Sealing state machine transitions
Windowed PoSt
mpool
Add and remove events are disabled by default, but this is how add events look.
Deals