Skip to content

hot restarter: Log errno for 'panic: cannot open shared memory' error#4032

Merged
mattklein123 merged 2 commits intoenvoyproxy:masterfrom
julia-stripe:log-errno-shared-memory
Aug 2, 2018
Merged

hot restarter: Log errno for 'panic: cannot open shared memory' error#4032
mattklein123 merged 2 commits intoenvoyproxy:masterfrom
julia-stripe:log-errno-shared-memory

Conversation

@julia-stripe
Copy link
Contributor

Description:

We've been seeing very frequent panic: cannot open shared memory region /envoy_shared_memory_160 check user permissions errors. This error message assumes that the reason the file can't be opened is because of permissions (EACCESS), but this can be misleading -- there are many reasons files might fail to open. In our case the cause was that the file didn't exist (ENOENT).

Logging the errno makes this particular error much easier to debug.

Risk Level: low
Testing: Ran in our QA cluster

The error message right now assumes that the errno is EACCESS which
isn't necessarily true. Logging the errno makes this easier to debug.

Signed-off-by: Julia Evans <julia@stripe.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM. Can you fix format? Thank you!

int shmem_fd = os_sys_calls.shmOpen(shmem_name.c_str(), flags, S_IRUSR | S_IWUSR);
if (shmem_fd == -1) {
PANIC(fmt::format("cannot open shared memory region {} check user permissions", shmem_name));
PANIC(fmt::format("cannot open shared memory region {} check user permissions. Error code: {}", shmem_name, errno));
Copy link
Member

Choose a reason for hiding this comment

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

Might also consider strerror() here, but not a big deal either way if you have a preference.

Signed-off-by: Julia Evans <julia@stripe.com>
@mattklein123 mattklein123 merged commit ddd661a into envoyproxy:master Aug 2, 2018
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