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

The historical view is not implemented for MemoryStore #2857

Closed
maschad opened this issue Jul 29, 2024 · 11 comments
Closed

The historical view is not implemented for MemoryStore #2857

maschad opened this issue Jul 29, 2024 · 11 comments
Labels
chore Issue is a chore

Comments

@maschad
Copy link
Member

maschad commented Jul 29, 2024

As we integrated launchTestNode across the whole monorepo, instances of the the error

FuelError: got error during work with storage The historical view is not implemented for `MemoryStore`

appear more frequently in test runs, which affects the consistency of our tests. This is obviously due to the fact that we are using an in-memory database, but using a persistent key-value store comes with it's own challenges, and there aren't any test cases where we actually require historical data yet, so I wouldn't suggest that as a solution.

We should at the very least handle this error as it's not relevant to our test cases, so that we reduce flakiness in our CI checks.

Related to #2698

Related to #1755

@maschad maschad added chore Issue is a chore p0 labels Jul 29, 2024
@maschad maschad assigned maschad and unassigned maschad Jul 29, 2024
@maschad maschad changed the title Fix issue with launchTestNode throwing the historical view is not implemented for MemoryStore Fix issue with launchTestNode throwing historical view is not implemented for MemoryStore Jul 29, 2024
@arboleya
Copy link
Member

What is there to be fixed on the ts-sdk?

@maschad
Copy link
Member Author

maschad commented Jul 30, 2024

@arboleya the error is being thrown here on random tests, so I've tried a very rudimentary way to suppress it in the tests since it's not relevant to our test cases, but I haven't investigated the issue in depth to see the root cause.

@petertonysmith94
Copy link
Contributor

Had a brief look and it appears to occur predominantly when we pass through a custom POA, however, this could be a red herring.

@arboleya
Copy link
Member

Following @Torres-ssf investigations, I understood it's coming from fuel-core.

If that is true, it may have nothing to do with launchTestNode, which makes this issue a bit misleading.

@petertonysmith94
Copy link
Contributor

Is this issue a P0?

@maschad
Copy link
Member Author

maschad commented Jul 30, 2024

@arboleya thanks for sharing that information, I'm still unsure how to reproduce the issue consistently. My primary rationale for raising this issue is that it's currently failing tests on #2811 and so I'll update the description according. At the very least we should handle the error / suppress it in test runs so it's not relevant to them.

@petertonysmith94 I've been experiencing recurring failing tests on #2811 and for that reason I've made it a p0

@maschad maschad changed the title Fix issue with launchTestNode throwing historical view is not implemented for MemoryStore Handle error thrown by launchTestNode when running tests Jul 30, 2024
@maschad maschad self-assigned this Jul 30, 2024
@maschad maschad added p2 and removed p0 labels Jul 30, 2024
@maschad
Copy link
Member Author

maschad commented Jul 30, 2024

I've lowered this to a p2 after further investigation, by skipping the main test causing this issue, this error can be avoided relatively consistently, although it can still appear.

@Torres-ssf
Copy link
Contributor

@maschad, I believe this error can be mitigated by using a longer --poa-interval-period when starting the node.

I am under the impression that this error tends to occur when the --poa-interval-period is set to a very short duration, such as a few milliseconds.

The reasoning is that a shorter --poa-interval-period causes nodes to be spawned more quickly, and since we are not using a persistent data solution, I suspect fuel-core eventually has trouble accessing data written in previous nodes, though I am not entirely sure about this.

@maschad
Copy link
Member Author

maschad commented Jul 31, 2024

@Torres-ssf I'm aware that the issue is related to --poa-interval-period as I had previously tried altering the period in previous commits but to no avail. Likewise I can't pinpoint the exact cause.

Unfortunately your recommendation is insufficient for this test use case (or any test similar to it) as it requires a transaction maturity > 1 and so we need a shorter interval period to increase block production time for such scenarios.

@arboleya arboleya removed the p2 label Aug 2, 2024
@maschad
Copy link
Member Author

maschad commented Aug 8, 2024

I'm unassigning myself from this for now as it would require substantial engineering effort and currently only one test would benefit from it. We can revisit this in the future if needs be.

@maschad maschad removed their assignment Aug 8, 2024
@arboleya arboleya changed the title Handle error thrown by launchTestNode when running tests The historical view is not implemented for MemoryStore Aug 8, 2024
@arboleya
Copy link
Member

arboleya commented Aug 8, 2024

Should be automatically solved when fuel-core releases this:

@maschad @Torres-ssf Please reopen if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issue is a chore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants