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

Fixed detecting number of NICs in EC2 #14252

Merged
merged 1 commit into from Feb 12, 2022

Conversation

merlimat
Copy link
Contributor

Motivation

In some EC2 instances we get an error when trying to read the NIC speed:

$ cat /sys/class/net/ens5/speed
cat: /sys/class/net/ens5/speed: Invalid argument

When that happens, we're ignoring that NIC and it causes that we cannot even manually override the NIC capacity in broker.conf, since the value that is configured is adjusted on the number of NICs. When we ignore that, the number gets then multiplied by 0.

@merlimat merlimat added type/bug The PR fixed a bug or issue reported a bug doc-not-needed Your PR changes do not impact docs release/2.9.3 release/2.8.4 labels Feb 11, 2022
@merlimat merlimat added this to the 2.10.0 milestone Feb 11, 2022
@merlimat merlimat self-assigned this Feb 11, 2022
@codelipenghui codelipenghui merged commit 7b169be into apache:master Feb 12, 2022
@merlimat merlimat deleted the fix-override-nic-speed-ec2 branch February 12, 2022 05:28
merlimat added a commit that referenced this pull request Feb 12, 2022
@merlimat merlimat added cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life labels Feb 12, 2022
// Check the type to make sure it's ethernet (type "1")
try {
String type = new String(Files.readAllBytes(path.resolve("type")), StandardCharsets.UTF_8).trim();
return Integer.parseInt(type) == 1;
Copy link
Member

Choose a reason for hiding this comment

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

@merlimat - by returning true here, in some cases, don't we also need to update the logic in getNicSpeedPath? We're seeing the new error here #14340 because this class is returning true in a new case.

Copy link
Member

Choose a reason for hiding this comment

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

When that happens, we're ignoring that NIC and it causes that we cannot even manually override the NIC capacity in broker.conf

I see now that the point was to make it so it could be overridden.

@michaeljmarshall
Copy link
Member

Given that this change can lead to new and verbose error logs, we should highlight in the release notes how to mitigate this error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants