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

chore: Minor Connection.Manager and Consumer cleanup and flake reduction #2305

Merged
merged 9 commits into from
Feb 6, 2025

Conversation

msfstef
Copy link
Contributor

@msfstef msfstef commented Feb 5, 2025

  • Removes ConnectionBackoff struct, scope is small and it's verbose (@robacourt nits are good we like them)
  • Uses the State struct everywhere - since we have it it makes sense to use it
  • Remove ShapeCache dependency in Consumer as it already has ShapeStatus available
  • Uses Consumer.whereis in the Consumer tests for allowing Mox assertions for consistency
  • Fixes the Consumer test flakes by waiting for the set_snapshot_started in the setup before running the tests, as the tests "override" some Mox expectations and allowances set in the setup if the snapshot is not ready before they get defined.

One thing that really worked for me that I hadn't tried before, is to use a tool like stress (e.g. stress -c 10) to max out the resources used by my machine while running tests to simulate a lower resource system that the tests on CI run on. I managed to consistently reproduce the consumer test flakes that way and debug them.

No changeset cause this is all refactoring and test fixing

@msfstef msfstef changed the title chore: Minor Connection.Manager cleanup and flake reduction chore: Minor Connection.Manager and Consumer cleanup and flake reduction Feb 5, 2025
@msfstef msfstef marked this pull request as ready for review February 5, 2025 17:50
Copy link
Contributor

@icehaunter icehaunter left a comment

Choose a reason for hiding this comment

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

Nice refactorings, love it! Please don't merge this until Garry's #2273 is in because I think there are conflicts that should go in first

@msfstef msfstef force-pushed the msfstef/minor-cleanup branch from 1864948 to fc4aaf7 Compare February 6, 2025 09:25
@robacourt
Copy link
Contributor

One thing that really worked for me that I hadn't tried before, is to use a tool like stress (e.g. stress -c 10) to max out the resources used by my machine while running tests to simulate a lower resource system that the tests on CI run on. I managed to consistently reproduce the consumer test flakes that way and debug them.

Great tip, thanks! I've added it here: https://electric-sql.slab.com/posts/sporadically-failing-elixir-tests-pfyia5on

@msfstef msfstef merged commit eaf0c29 into main Feb 6, 2025
34 checks passed
@msfstef msfstef deleted the msfstef/minor-cleanup branch February 6, 2025 09:39
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.

3 participants