Skip to content
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

fix(events): check for sync-in-progress #11932

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Apr 26, 2024

We use currentHeight to figure out where we are to know whether we need to backfill a filter or not. Unfortunately, we collect this from Apply() so if you install a filter prior to processing a message, we can't know where we are and what we should do about it.

The current behaviour is that it will fail the minHeight < currentHeight test further down in this file, not perform a pre-fill, and we end up with an empty filter (initially). So any calls to GetActorEventsRaw or SubscribeActorEventsRaw wanting backfill will not get any of it and will return a null to the user. But, this behaviour is the same as "there are no events in this range", so the user has no way of differentiating between "node is starting" and "no events".

This new behaviour will send an error in response to a GetActorEventsRaw or SubscribeActorEventsRaw or even eth_getLogs if the node is still starting and we haven't seen our first Apply.

An alternative to this behaviour is to go ask the chain what the current head is, but I don't think we currently have the means to do this in EventFilterManager, we only have a ChainStore to work with.

@ribasushi
Copy link
Collaborator

ask the chain what the current head is, but I don't think we currently have the means to do this in EventFilterManager, we only have a ChainStore to work with.

is only have a ChainStore a misphhrase? Because if not - it is the place that knows about the current head: https://github.com/filecoin-project/lotus/blob/v1.26.3/chain/store/store.go#L983-L989

@rvagg
Copy link
Member Author

rvagg commented Apr 26, 2024

Derp, I looked and couldn't see an obvious way to get it; I didn't look long enough! Thanks @ribasushi.

Updated the PR to update m.currentHeight instead of erroring if we haven't recorded it yet.

Copy link
Contributor

@aarshkshah1992 aarshkshah1992 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rvagg rvagg merged commit ae2cc67 into master Apr 29, 2024
186 checks passed
@rvagg rvagg deleted the rvagg/sync-in-progress-events branch April 29, 2024 02:19
@rvagg
Copy link
Member Author

rvagg commented Apr 29, 2024

@rjan90 this would be good to get into the next release if possible

snadrus added a commit that referenced this pull request May 2, 2024
* Fixing dead links (#11907)

* ci: ci: create gh workflow that updates sorted pr checks (#11861)

* ci: create gh workflow that updates sorted pr checks

* ci: use grouped_by_result template for pr checks sticky comment

* chore: apply pr review suggestion

* Avoid cfg lookup on chain remove since unenabled splitstore delete is noop anyway (#11916)

Co-authored-by: zenground0 <[email protected]>

* Fix mismatched method names in comments (#11913)

Signed-off-by: forcedebug <[email protected]>

* release: v1.26.3 (#11908) (#11915) (#11922)

* deps: update dependencies to address migration memory bloat

to address memory concerns during a heavy migration

Ref: filecoin-project/go-state-types#260
Ref: whyrusleeping/cbor-gen#96
Ref: filecoin-project/go-amt-ipld#90

* release: prep v1.26.3 patch

Prep v1.26.3 patch release:
- Update changelog, version and make gen + make docsgen-cli

* deps: update cbor-gen to tagged version

deps: update cbor-gen to tagged version

* deps: update go-state-types to tagged version

deps: update go-state-types to tagged version v0.13.2

* chore: deps: update go-state-types to v0.13.3

Fixes a panic when we have fewer than 1k proposals.

---------

Co-authored-by: Phi-rjan <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Steven Allen <[email protected]>

* Refactor `LookupID*` APIs in `StateManager` and `StateTree`

The naming of `LookupID` can cause confusion when resolving actor IDs vs
 ID addresses. To avoid this:

* Refactor `StateTree` `LookupID` to `LookupIDAddress`, because it
returns ID address.
* Refactor `StateManager` `LookupID` to
`LookupIDAddress` because it also returns ID address via a chain call to
`StateTree`.
* Introduce a new API `StateManager` dedicated to resolving address to
actor ID, called `LookupID` which returns `abi.ActorID`.

For context, see:
 * #11723 (comment)

* Add v13 support to invariants-checker (#11931)

Add v13 support to invariants-checker

* chore: docs: nv-skeleton documentation (#11065)

* nv-skeleton documentation

Add a tutorial for how one can create a nv-skeleton in Lotus

* Add footnote for `Add migration` step

Add footnote for `Add migration` step

* Indent migration-code

Indent migration-code to make it show properly as a footnote.

* Add ref-fvm and filecoin-ffi checklist

Add ref-fvm and filecoin-ffi checklist

* Add Filecoin-FFI steps

Add Filecoin-FFI steps

* Add step to params_butterfly.go

Add step to params_butterfly.go

* Fix typo

Fix typo

* Add links to reference PRs

Add links to reference PRs

* Update ref-fvm list

Update ref-fvm list

* feat: curio: add StorageInit api (#11918)

* feat: add StorageInit api

* remove unused variables

* fix gen check

* feat: curio: simpler reservation release logic (#11900)

* simpler release logic

* oops, plus simpler

* simpler

* fix NewLine (#11893)

* fix(events): check for sync-in-progress (#11932)

* feat(events): adjust indexes in event index db to match query patterns

Introduces a v4 migration that just adjusts indexes.

Copies some improvements from #11723

Closes: #11909

* fix(pipeline): should return if error occurred when get network version (#11902)

* fix(events): correct log msg for v4 events index db migration

* chore: remove duplicate words in strings and comments

* fix(events): register events index db migration v4

* fix: curio seal: Failed commit retry strategy (#11870)

* ffi: improved-error-handling

* curio seal: Failed commit retry strategy

* use master ffi

* mod tidy

* fix: curio: Update pgx imports, fix db_storage alloc

* feat: curioweb: Improve task_history indexes (#11911)

* mod tidy

* Event index should be unique for tipsets (#11952)

* event index should be unique for tipsets

* fix formatting

* migrate to version 5

* chore: bump build version in master (#11946)

* Bump version

Bump version in master branch in preperation for cutting v1.27.0-rc1

* chore: bump build-version

chore: bump build-version

* feat: curioweb: Show piece info on the sector page (#11955)

* curio: feat: break trees task into TreeD(prefetch) and TreeRC (#11895)

* break trees task

* fix TreeD reservation

* fix nil pointer err

* apply suggestions

* fix allocate file types

* fix dbIndex inserts

* set resource, move release func

* refactor func(), update memory

* remove extra release

---------

Signed-off-by: forcedebug <[email protected]>
Co-authored-by: parthshah1 <[email protected]>
Co-authored-by: Piotr Galar <[email protected]>
Co-authored-by: ZenGround0 <[email protected]>
Co-authored-by: zenground0 <[email protected]>
Co-authored-by: forcedebug <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
Co-authored-by: Phi-rjan <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: Lee <[email protected]>
Co-authored-by: Andrew Jackson (Ajax) <[email protected]>
Co-authored-by: beck <[email protected]>
Co-authored-by: 0x5459 <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: Aarsh Shah <[email protected]>
magik6k added a commit that referenced this pull request May 4, 2024
* fix: curio: Update pgx imports, fix db_storage alloc

* feat: curioweb: Improve task_history indexes (#11911)

* 1

* relatable

* add and delete layer

* chore: curio: merge master (#11956)

* Fixing dead links (#11907)

* ci: ci: create gh workflow that updates sorted pr checks (#11861)

* ci: create gh workflow that updates sorted pr checks

* ci: use grouped_by_result template for pr checks sticky comment

* chore: apply pr review suggestion

* Avoid cfg lookup on chain remove since unenabled splitstore delete is noop anyway (#11916)

Co-authored-by: zenground0 <[email protected]>

* Fix mismatched method names in comments (#11913)

Signed-off-by: forcedebug <[email protected]>

* release: v1.26.3 (#11908) (#11915) (#11922)

* deps: update dependencies to address migration memory bloat

to address memory concerns during a heavy migration

Ref: filecoin-project/go-state-types#260
Ref: whyrusleeping/cbor-gen#96
Ref: filecoin-project/go-amt-ipld#90

* release: prep v1.26.3 patch

Prep v1.26.3 patch release:
- Update changelog, version and make gen + make docsgen-cli

* deps: update cbor-gen to tagged version

deps: update cbor-gen to tagged version

* deps: update go-state-types to tagged version

deps: update go-state-types to tagged version v0.13.2

* chore: deps: update go-state-types to v0.13.3

Fixes a panic when we have fewer than 1k proposals.

---------

Co-authored-by: Phi-rjan <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Steven Allen <[email protected]>

* Refactor `LookupID*` APIs in `StateManager` and `StateTree`

The naming of `LookupID` can cause confusion when resolving actor IDs vs
 ID addresses. To avoid this:

* Refactor `StateTree` `LookupID` to `LookupIDAddress`, because it
returns ID address.
* Refactor `StateManager` `LookupID` to
`LookupIDAddress` because it also returns ID address via a chain call to
`StateTree`.
* Introduce a new API `StateManager` dedicated to resolving address to
actor ID, called `LookupID` which returns `abi.ActorID`.

For context, see:
 * #11723 (comment)

* Add v13 support to invariants-checker (#11931)

Add v13 support to invariants-checker

* chore: docs: nv-skeleton documentation (#11065)

* nv-skeleton documentation

Add a tutorial for how one can create a nv-skeleton in Lotus

* Add footnote for `Add migration` step

Add footnote for `Add migration` step

* Indent migration-code

Indent migration-code to make it show properly as a footnote.

* Add ref-fvm and filecoin-ffi checklist

Add ref-fvm and filecoin-ffi checklist

* Add Filecoin-FFI steps

Add Filecoin-FFI steps

* Add step to params_butterfly.go

Add step to params_butterfly.go

* Fix typo

Fix typo

* Add links to reference PRs

Add links to reference PRs

* Update ref-fvm list

Update ref-fvm list

* feat: curio: add StorageInit api (#11918)

* feat: add StorageInit api

* remove unused variables

* fix gen check

* feat: curio: simpler reservation release logic (#11900)

* simpler release logic

* oops, plus simpler

* simpler

* fix NewLine (#11893)

* fix(events): check for sync-in-progress (#11932)

* feat(events): adjust indexes in event index db to match query patterns

Introduces a v4 migration that just adjusts indexes.

Copies some improvements from #11723

Closes: #11909

* fix(pipeline): should return if error occurred when get network version (#11902)

* fix(events): correct log msg for v4 events index db migration

* chore: remove duplicate words in strings and comments

* fix(events): register events index db migration v4

* fix: curio seal: Failed commit retry strategy (#11870)

* ffi: improved-error-handling

* curio seal: Failed commit retry strategy

* use master ffi

* mod tidy

* fix: curio: Update pgx imports, fix db_storage alloc

* feat: curioweb: Improve task_history indexes (#11911)

* mod tidy

* Event index should be unique for tipsets (#11952)

* event index should be unique for tipsets

* fix formatting

* migrate to version 5

* chore: bump build version in master (#11946)

* Bump version

Bump version in master branch in preperation for cutting v1.27.0-rc1

* chore: bump build-version

chore: bump build-version

* feat: curioweb: Show piece info on the sector page (#11955)

* curio: feat: break trees task into TreeD(prefetch) and TreeRC (#11895)

* break trees task

* fix TreeD reservation

* fix nil pointer err

* apply suggestions

* fix allocate file types

* fix dbIndex inserts

* set resource, move release func

* refactor func(), update memory

* remove extra release

---------

Signed-off-by: forcedebug <[email protected]>
Co-authored-by: parthshah1 <[email protected]>
Co-authored-by: Piotr Galar <[email protected]>
Co-authored-by: ZenGround0 <[email protected]>
Co-authored-by: zenground0 <[email protected]>
Co-authored-by: forcedebug <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
Co-authored-by: Phi-rjan <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: Lee <[email protected]>
Co-authored-by: Andrew Jackson (Ajax) <[email protected]>
Co-authored-by: beck <[email protected]>
Co-authored-by: 0x5459 <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: Aarsh Shah <[email protected]>

* linter oops

* gen cleanup

* fix

* named returns are confusing

---------

Signed-off-by: forcedebug <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: LexLuthr <[email protected]>
Co-authored-by: parthshah1 <[email protected]>
Co-authored-by: Piotr Galar <[email protected]>
Co-authored-by: ZenGround0 <[email protected]>
Co-authored-by: zenground0 <[email protected]>
Co-authored-by: forcedebug <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
Co-authored-by: Phi-rjan <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: Lee <[email protected]>
Co-authored-by: beck <[email protected]>
Co-authored-by: 0x5459 <[email protected]>
Co-authored-by: Aarsh Shah <[email protected]>
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request May 11, 2024
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants