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

Move NeighborInfo interval default to 6 hours (double NodeInfo) #4483

Merged
merged 3 commits into from
Aug 17, 2024

Conversation

thebentern
Copy link
Contributor

@thebentern thebentern commented Aug 17, 2024

As a part of the default MQTT broker traffic management, we've noticed a disproportionate percentage of packets lately are NeighborInfo. Given that this data is really only useful for mesh topology visualizations, I'm proposing moving it to a 6-hour interval, which is double that of its most similar companion: NodeInfo.

@thebentern thebentern requested a review from GUVWAF August 17, 2024 14:08
@garthvh
Copy link
Member

garthvh commented Aug 17, 2024

Do 6, twice a day is fine

@thebentern
Copy link
Contributor Author

I agree. Sounds good. So we'll double both the node info broadcast interval default and minimum allowed.

@thebentern thebentern changed the title Move NeighborInfo interval default to 3 hours, in line with NodeInfo Move NeighborInfo interval default to 6 hours (double NodeInfo) Aug 17, 2024
@thebentern thebentern merged commit 5ff1078 into master Aug 17, 2024
100 checks passed
@jp-bennett
Copy link
Collaborator

@thebentern Take a second look at this. I think you accidentally dropped the ms to second scaling, so Neighborinfo happens about 1000 times more often than intended.

@Talie5in
Copy link
Contributor

@thebentern Take a second look at this. I think you accidentally dropped the ms to second scaling, so Neighborinfo happens about 1000 times more often than intended.

Can confirm, opened a Issue
#4502

@thebentern thebentern deleted the neighbor-info-interval branch August 19, 2024 11:51
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.

4 participants