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 nested nodes in commits #385

Merged
merged 9 commits into from
Feb 14, 2023
Merged

Fix nested nodes in commits #385

merged 9 commits into from
Feb 14, 2023

Conversation

cap10morgan
Copy link
Contributor

Fixes #380

The whole fix ended up being 12b59bc. The rest of the commits are small things I or @bplatz noticed along the way.

And since I had to lookup how to run one test in isolation for this, I added a note to the README about it.

@cap10morgan cap10morgan requested a review from a team February 14, 2023 19:41
Copy link
Contributor

@zonotope zonotope left a comment

Choose a reason for hiding this comment

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

🧾

{:keys [flakes pid sid]} (<? (assert-flakes db assert t-new iris refs))
all-flakes (-> (empty (get-in db [:novelty :spot]))
(into (<? retract-flakes))
(into retract-flakes)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know the <? for retracts looked misplaced as it was, but it was intentional.

Its purpose was to run the fairly costly assert-flakes and retract-flakes in parallel on different threads, as opposed to serially.

This change makes it run serially.

Copy link
Contributor Author

@cap10morgan cap10morgan Feb 14, 2023

Choose a reason for hiding this comment

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

I made the original parallelization change here 3 weeks ago (in 64a7fd7) but recently noticed that it may not be safe b/c it's being passed a volatile map that is also being passed to assert-flakes.

So I reverted it back to the way it was before 64a7fd7.

@cap10morgan cap10morgan merged commit 1f7ef75 into main Feb 14, 2023
@cap10morgan cap10morgan deleted the fix/issue-380 branch February 14, 2023 20:32
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.

Strange query results from a loaded ledger vs. live value
3 participants