openstreetmap / openstreetmap/mod_tile

DEBUG: Failed to read cmd on fd (2)

Open
#170 0 comments 11 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
318
Forks
199
PR merge metrics
No merged PRs in 30d

Description

Sorry for duplicating this issue, but the original one #77 became messy as there were discussed not-related things, including "doesn't work" problems. This one is dedicated only to a DEBUG message which you can see in your logs, e.g.:

renderd[17]: DEBUG: Failed to read cmd on fd 4
renderd[17]: DEBUG: Sending render cmd(3 osmcarto 8/159/93) with protocol version 2 to fd 8
renderd[17]: DEBUG: Connection 0, fd 4 closed, now 5 left
renderd[17]: DEBUG: Failed to read cmd on fd 11
renderd[17]: DEBUG: Connection 4, fd 11 closed, now 4 left
renderd[17]: DEBUG: Failed to read cmd on fd 8
renderd[17]: DEBUG: Sending render cmd(3 osmcarto 8/159/92) with protocol version 2 to fd 7
renderd[17]: DEBUG: Connection 1, fd 8 closed, now 3 left
renderd[17]: DEBUG: Failed to read cmd on fd 10
renderd[17]: DEBUG: Connection 2, fd 10 closed, now 2 left
renderd[17]: DEBUG: Failed to read cmd on fd 7
renderd[17]: DEBUG: Connection 0, fd 7 closed, now 1 left
renderd[17]: DEBUG: Failed to read cmd on fd 9
renderd[17]: DEBUG: Connection 0, fd 9 closed, now 0 lef

According the this reply https://github.com/openstreetmap/mod_tile/issues/77#issuecomment-50244923 which I quote here:

The "failed to read cmd" message was added by Kai in 9d4e10e. It will be triggered any time a connection is closed because recv() returns 0 for this case. I don't think it is any cause for concern but maybe we want to set this at LOG_DEBUG or differentiate between -1 (errors) and 0 (successful close).

-- there is even nothing to worry about.

But let's make it clear: if you print a message like that into a log, they you assume it's to be seen by a sysadmin and some actions to be done, correct? Anyway, such fail-messages are always making us a bit nervous. I didn't get into details, but just looking at this code:

    ret = recv(fd, cmd, sizeof(struct protocol_v1), block?MSG_WAITALL:MSG_DONTWAIT);
    if (ret < 1) {
        syslog(LOG_INFO, "DEBUG: Failed to read cmd on fd %i", fd);
        return -1;
    } else if (ret < sizeof(struct protocol_v1)) {
        syslog(LOG_INFO, "DEBUG: Read incomplete cmd on fd %i", fd);
        return 0;
    }

-- I assume that getting ret < 1 is not what this code "treats" as normal instantly exiting the function. It'd rather want to see some other value and continue the execution flow. But if contrariwise exiting the function is totally normal (sort of EOF) then why print this message after all? Who needs it and what is supposed to do? Does it sign about some bad situation at server-side or client-side or not at all? Please clarify.

What I want to say is:

  • if this message really doesn't signal about internal errors
    OR
  • if it doesn't affect caching
    OR
  • if it doesn't affect performance
    THEN
  • this message could be probably simply removed.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the recv() handling shown in the issue and trace how a return value below 1 is treated before the connection closes. Check whether a normal client disconnect is being logged as a failure and whether the message affects caching or performance. Done means the log accurately distinguishes normal closure from an error, or the misleading message is removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.