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

Rebase to Horde 0.9.0 #8

Merged
merged 0 commits into from
Oct 2, 2024
Merged

Rebase to Horde 0.9.0 #8

merged 0 commits into from
Oct 2, 2024

Conversation

studzien
Copy link
Collaborator

@studzien studzien commented Oct 2, 2024

This rebases our changes to Horde 0.9.0 (the latest version).

Two additional changes:

  1. This PR was reverted -- 8fbaf33
    This change was incomplete and incorrect in the scenario when a process crashed on the same host. The restarted process might not have been re-added to the registry.
  2. From horde's master - this PR was reverted - 0d8c9ae
    That's because we still care about the synchronicity (the PR mentioned in point 1) of the auction process start (that's what OTP's processes rely on), but it's not susceptible to the issue introduced in Add process added to local Register synchronously #2. We also don't care about the scenario that the PR fixed initially because we don't start two processes with the same key.

When this is approved, I will force push to master.

@PranavVadrevu
Copy link

Just for my understanding:

This change was incomplete and incorrect in the scenario when a process crashed on the same host. The restarted process might not have been re-added to the registry

Why would the restarted process not be added to the registry if doing this synchronously @studzien?

@studzien
Copy link
Collaborator Author

studzien commented Oct 2, 2024

Just for my understanding:

This change was incomplete and incorrect in the scenario when a process crashed on the same host. The restarted process might not have been re-added to the registry

Why would the restarted process not be added to the registry if doing this synchronously @studzien?

So I believe the problem was here, so essentially when another add operation was processed on the same node, it was ignored. I can't remember exactly, but I guess I wrongly assumed that handling this in register only would do the job.

I tested this by running this test:
https://github.com/Whatnot-Inc/whatnot_live/blob/main/test/medium/da/auction_test.exs#L408-L436
with and without sleep.

@studzien studzien merged commit 0d8c9ae into master Oct 2, 2024
5 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants