Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Revert enabling authority discovery by default #1532

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Aug 4, 2020

Reverts #1395.

The authority discovery module ends up consuming a very high amount of sockets which could lead to exhaustion of file descriptors.

Screenshot_2020-08-04_10-54-01

Here's the difference on my local node with and without authority discovery enabled. On the left we can see peaks of >4k sockets and low is at ~1.5k. Without authority discovery we still see an initial peak (presumably due to regular DHT discovery) but the absolute number of sockets taken is an order of magnitude lower.

@andresilva andresilva added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. B0-silent Changes should not be mentioned in any release notes labels Aug 4, 2020
@andresilva andresilva force-pushed the andre/disable-authority-discovery branch from 880ee8d to 3bd081b Compare August 4, 2020 15:21
@bkchr
Copy link
Member

bkchr commented Aug 5, 2020

Do we still want this?

@rphmeier rphmeier merged commit 671cc75 into master Aug 5, 2020
@rphmeier rphmeier deleted the andre/disable-authority-discovery branch August 5, 2020 12:10
@tomaka
Copy link
Contributor

tomaka commented Aug 5, 2020

So what is the criteria for re-enabling authority-discovery again? For all we know it's working as intended, and I don't know what would need to change.

ordian added a commit that referenced this pull request Aug 5, 2020
* master:
  [CI] Fix draft release publishing (#1546)
  Bump Substrate, version (#1541)
  Update check_labels runtimenoteworthy label (#1540)
  revert enabling authority discovery by default (#1532)
mxinden added a commit that referenced this pull request Oct 12, 2020
This reverts commit 671cc75.

The authority discovery module was initially enabled by default on
validator and sentry nodes with commit bc6e1e7. This change was later on
reverted in 671cc75. With this commit the authority discovery module is
again enabled by default.
ghost pushed a commit that referenced this pull request Oct 13, 2020
This reverts commit 671cc75.

The authority discovery module was initially enabled by default on
validator and sentry nodes with commit bc6e1e7. This change was later on
reverted in 671cc75. With this commit the authority discovery module is
again enabled by default.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants