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

[BUG] Cached(refresh = true) will collect SoT reader twice #572

Open
sky-ricardocarrapico opened this issue Jul 31, 2023 · 3 comments
Open
Labels
bug Something isn't working

Comments

@sky-ricardocarrapico
Copy link

Describe the bug
I'm not completely sure this is a bug, but I can't possibly think of a reason for it to not be a bug, so I'd like to see some confirmation.

When requesting with StoreReadRequest.cached(key = key, refresh = true), SoT reader will get hit twice. This will effectively restart any DB observation and run queries again.

While on most situations this shouldn't be an issue, I have a very particular case where I'm leveraging the scope of the reader flow (using channelFlow) to do some operations while there is someone still collecting it. With this behavior of collecting twice, my scope is getting cancelled and so are the operations.

I really can't think of a reason for it to need to collect twice. I'd hope that when we start .stream, it would collect reader and just stay there.

Using StoreReadRequest.cached(key = key, refresh = false) works without issue.

Is it really a bug? Thanks!

To Reproduce
See above.

Expected behavior
With only one call to .stream, reader should only be collected once.

Screenshots
N/A

Smartphone (please complete the following information):

  • Device: N/A
  • OS: N/A
  • Store Version 5.0.0-beta02

Additional context
N/A

@sky-ricardocarrapico sky-ricardocarrapico added the bug Something isn't working label Jul 31, 2023
@github-project-automation github-project-automation bot moved this to 🆕 Triage in Store Roadmap Jul 31, 2023
@digitalbuddha
Copy link
Contributor

I don't believe this was intentional. I'll take a look before next release. Happy to accept a PR if you want to investigate yourself.

@kakai248
Copy link

kakai248 commented Aug 19, 2023

So I investigated what's happening. Wrote a failing test here: 3f05bfa

This is all due to how SourceOfTruthWithBarrier works, by blocking reads when a write is happening. But this "block" stops collecting SoT reader and restarts after the write the done.

What is the reason we need this mechanism? Would a debounce work here instead? We keep collecting but discard all values while writing and when it stops we only let the last one pass.

@digitalbuddha
Copy link
Contributor

Oh gosh this has been a few years. I think the thinking here is that we wanted to make the response origin of the next read as being from network rather than disk. To prevent races we disabled reading until after the write succeeded. I would imagine filtering reads out until after the right succeeeds would give similar functionality. Happy to accept a PR if you'd like to take a stab at it.

Not sure if @yigit remembers anything better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 🆕 Triage
Development

No branches or pull requests

3 participants