-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add miner_sector_posts tracking of window posts #74
Conversation
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.
Minor suggestions. 🤝
Co-authored-by: Mike Greenberg <[email protected]>
model/actors/miner/sectorposts.go
Outdated
return nil | ||
} | ||
|
||
// TODO: what makes 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 would start by looking here: https://github.com/filecoin-project/sentinel-visor/blob/master/tasks/actorstate/miner.go#L26, knee jerk reaction is to make it here: https://github.com/filecoin-project/sentinel-visor/blob/master/model/actors/miner/task.go#L41
storage/migrations/5_windowpost.go
Outdated
@@ -0,0 +1,24 @@ | |||
package migrations |
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'll want to make this migration 6 after a rebase I believe.
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.
It'll be 8 now. This is going to continue to be a problem while we have an integer index for migrations. Perhaps it would be better to use timestamps to make the index collisions (at least between migrations touching mutually exclusive schema) less painful to reconcile. But maybe this is a short term problem while dev velocity is high. Just a showerthought.
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.
The migrations package we use expects a monotonic counter. It will fail if there are two migrations for the same schema version. I'll add a test for that so we see it at build time instead of migration time.
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.
Good stuff, have you been able to manually verify this works against a lotus node and database?
if msg.To == actor.Address && msg.Method == 5 /* miner.SubmitWindowedPoSt */ { | ||
if err := processPostMsg(msg); err != nil { | ||
return nil, err | ||
} | ||
} | ||
} | ||
for _, msg := range msgs.SecpkMessages { | ||
if msg.Message.To == actor.Address && msg.Message.Method == 5 /* miner.SubmitWindowedPoSt */ { |
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 am hesitant to rely on miner.SubmitWindowedPoSt
always being equal to 5
but I don't have any ideas on how to avoid this... and would consider trying to extract the method numbers from here: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/methods.go#L83
* origin/master: fix: remove errgrp from UnindexedBlockData persist fix: verify there are no missing migrations before migrating (#89) chore: add tests for reward and power actor state extracters (#83) feat: support v2 actor codes (#84) feat(debug): Process actor by head without persistance (#86) fix: use debugf logging method in message processor (#82) feat(task): Create chainvis views and refresher (#77) perf: minor optimization of market actor diffing (#78) chore: fail database tests if VISOR_TEST_DB not set (#79) test(storage): add test to check for duplicate schema migrations (#80) in-process lotus "repo" lens (#75) Conflicts: tasks/actorstate/miner.go
* origin/master: feat: Add historical indexer metrics (#92) fix(lens): Include dependencies needed for Repo Lens (#90) Move migration to schema version 10 fix(migrations): migrations require version 0 feat: add message gas economy processing perf(db): reduce batch size for chain history indexer (#105) fix: use hash index type for visor_processing_actors_code_idx (#106) feat: set application name in postgres connection (#104) feat: add visor processing stats table (#96) feat(task): add chain economics processing (#94) get actor name for both versions of specs-actors (#101)
…-view * origin/master: feat: Add miner_sector_posts tracking of window posts (#74) feat: Add historical indexer metrics (#92) fix(lens): Include dependencies needed for Repo Lens (#90) Move migration to schema version 10 fix(migrations): migrations require version 0 feat: add message gas economy processing perf(db): reduce batch size for chain history indexer (#105) fix: use hash index type for visor_processing_actors_code_idx (#106) feat: set application name in postgres connection (#104) feat: add visor processing stats table (#96) feat(task): add chain economics processing (#94) get actor name for both versions of specs-actors (#101) fix: remove errgrp from UnindexedBlockData persist
Address #61