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

Issue 102 #103

Merged
merged 2 commits into from
Nov 5, 2021
Merged

Issue 102 #103

merged 2 commits into from
Nov 5, 2021

Conversation

BobEvans
Copy link
Collaborator

@BobEvans BobEvans commented Nov 4, 2021

Fixes #102

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to README are included in PR

@rundong08 rundong08 changed the base branch from develop to issue_100 November 4, 2021 23:29
@rundong08 rundong08 changed the base branch from issue_100 to develop November 4, 2021 23:29
Copy link
Collaborator

@rundong08 rundong08 left a comment

Choose a reason for hiding this comment

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

Thanks! It LGTM, although it seems still possible to have race conditions when multiple write happens. But I suspect fixing that would require a redesign of taqo_shared_prefs. And since in practice this PR fixes existing issues without triggering new issues yet. I'll just merge this PR.

@rundong08 rundong08 merged commit a6617c1 into develop Nov 5, 2021
@BobEvans
Copy link
Collaborator Author

BobEvans commented Nov 5, 2021

Thanks!

One easy thought is that we could make the calls synchronous so that the two await calls inside the method go away. Then other callers would be blocked on writing. Given the amount of data we are processing, it might not be a bottleneck and it would be safe. Am I missing anything?

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.

trigger not respecting minimum buffer
2 participants