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 monitor exclusion not being applied for newly detected nodes #392

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

KiKoS0
Copy link
Contributor

@KiKoS0 KiKoS0 commented Nov 24, 2024

Closes #388

Summary

Fixes a small copy-paste typo making the monitor_excludes option not being used at all and the monitor_includes option being wrongly used in the Ring router to exclude newly detected nodes instead.

Details

I was having the same issue of what #388 is describing and noticed that it was caused by remote accessing nodes. When a remote shell connects to an app's node, it doesn't get excluded by the default monitor exclusions defined in:

monitor_excludes =
Options.get(options, :monitor_excludes, &is_list/1, [
~r/^remsh.*$/,
~r/^rem-.*$/
])

You can see that the rem- values are showing up in this comment when calling Cachex.Router.connected()

iex(core@10.244.1.158)3> Cachex.Router.connected()
[:"[email protected]", :"[email protected]", :"[email protected]"]

iex(core@10.244.2.34)15> Cachex.Router.connected()
[:"[email protected]", :"[email protected]", :"[email protected]"]

Small copy-paste typo making the `monitor_excludes` option not being
used at all and the `monitor_includes` option being used to exclude
nodes instead.
@whitfin
Copy link
Owner

whitfin commented Nov 24, 2024

Hi @KiKoS0!

Awesome find, thank you so much for taking the time to look into it. I'll double check the CI/CD runs, and assuming it's all good I'll push this out immediately as a patch!

@whitfin whitfin self-assigned this Nov 24, 2024
@whitfin whitfin added the bug label Nov 24, 2024
@whitfin whitfin merged commit 24ba777 into whitfin:main Nov 24, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

:ets.lookup error from Cachex Overseer Table
2 participants