Timeout Select errors by up to 1s to avoid callers pinning CPU and filling /var/log

[mdhoff-ms]

I have noticed two instances of filled /var/log on DUTs with many repeated lines:

2026 Jul 21 01:47:00.605758 ERR caclmgrd: :- poll_descriptors: readData error: Unable to read redis reply
2026 Jul 21 01:47:00.605776 ERR healthd: :- poll_descriptors: readData error: Unable to read redis reply
2026 Jul 21 01:47:00.605793 ERR featured: :- poll_descriptors: readData error: Unable to read redis reply

these repeat immediately, leading to /var/log partition being filled very quickly. It seems that the error case of common/select.cpp (e.g. if redis is down) returns immediately, leading to callers who call in loops spinning indefinitely and filling the log partition.

To fix this I propose the following change:
On the Select::ERROR path, we should enforce the timeout requested by the caller (often 1ms, 100ms, etc) up to a cap of 1 second, to avoid pinning the CPU in a hot loop. Additionally the readData error log should be rate limited to once per second, to avoid flooding /var/log/syslog and filling the partition.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates the core swss::Select event loop implementation to throttle repeated Select::ERROR returns, reducing CPU pinning and runaway log growth when underlying selectables (e.g., Redis-backed ones) repeatedly fail to readData().

Changes:

• Adds a 1-second backoff when Select::select() returns Select::ERROR.
• Refactors the early-return logic in Select::select() to make return conditions more explicit.
• Adds <chrono> / <thread> includes to support the new throttling sleep.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[mdhoff-ms]

maybe a good idea is to sleep only for the timeout that's passed in, then add a separate backoff (equal to 1s) to the actual logging statement. This would preserve the exact contract the caller is expecting while also mitigating the log spam.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[qiluo-msft]

This is caller's responsibility to handle ERROR gracefully, not to burn CPU.

[qiluo-msft]

Suggest not to add clock check. This function is critical to object sync performance.

[mdhoff-ms]

I have swapped this to a cap on total consecutive failure logs. After thinking about it, I actually prefer this to the clock method also.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

common/select.cpp:205

• Select::select() only applies the error backoff when the initial non-blocking poll_descriptors(..., 0) returns ERROR. If the first poll returns TIMEOUT and the subsequent poll_descriptors(..., timeout, ...) returns ERROR (e.g., readData throws after epoll_wait wakes), the function returns immediately with ERROR and callers can still spin in a tight loop, which defeats the PR’s goal.

    } else if (ret == Select::TIMEOUT)
    {
        ret = poll_descriptors(c, timeout, interrupt_on_signal);
    } else if (ret == Select::ERROR)
    {

common/select.cpp:147

• The PR description says the readData error log should be rate-limited to once per second, but this implementation instead logs up to 10 consecutive errors and then suppresses all further errors until any successful read occurs. Also, because the suppression counter is thread_local and not keyed per Select/Selectable, errors from one failing selectable can suppress logging for unrelated selectables in the same thread.

This issue also appears on line 201 of the same file.

            if (s_consecutiveErrors < kMaxConsecutiveErrorLogs)
            {
                s_consecutiveErrors++;
                SWSS_LOG_ERROR("readData error: %s%s", ex.what(),
                    s_consecutiveErrors == kMaxConsecutiveErrorLogs

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

common/select.cpp:149

• PR description says the readData error log should be rate-limited (once per second), but the implementation here permanently suppresses all further logs after 10 consecutive errors until a successful read occurs. On a long-lived outage this can hide the ongoing failure entirely after the first 10 messages, which is materially different behavior than rate limiting.

Consider implementing a time-based throttle (e.g., thread_local last-log timestamp; log at most once per second, optionally with a suppressed-count summary), or update the PR description/tests to match the intended ‘cap then silence’ semantics.

            if (s_consecutiveErrors < kMaxConsecutiveErrorLogs)
            {
                s_consecutiveErrors++;
                SWSS_LOG_ERROR("readData error: %s%s", ex.what(),
                    s_consecutiveErrors == kMaxConsecutiveErrorLogs

common/select.cpp:217

• The new ERROR-path backoff (sleep_for) is the main behavioral change intended to prevent tight loops from pinning CPU, but the added unit tests only validate log suppression. A regression here (e.g., removing/shortening the sleep) would not be caught.

Consider adding a unit test that measures elapsed time for repeated ERROR returns with a non-zero timeout and asserts there is a minimum backoff (with some slack to avoid flakiness).

        // ERROR returns immediately, so a tight caller loop pins the CPU. Back off
        // by the timeout capped at 1s; INFINITE (< 0) counts as the largest timeout.
        constexpr int kMaxErrorBackoffMs = 1000;
        int backoffMs = (timeout < 0 || timeout > kMaxErrorBackoffMs) ? kMaxErrorBackoffMs : timeout;
        std::this_thread::sleep_for(std::chrono::milliseconds(backoffMs));

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

common/select.cpp:218

• select() sleeps on Select::ERROR before returning, but poll_descriptors() may already have queued other healthy selectables into m_ready earlier in the same epoll batch. Sleeping unconditionally here can delay processing of already-ready data by up to the backoff amount. Consider only applying the backoff sleep when there is no queued ready work (m_ready.empty()).

        // ERROR returns immediately, so a tight caller loop pins the CPU. Back off
        // by the timeout capped at 1s; INFINITE (< 0) counts as the largest timeout.
        constexpr int kMaxErrorBackoffMs = 1000;
        int backoffMs = (timeout < 0 || timeout > kMaxErrorBackoffMs) ? kMaxErrorBackoffMs : timeout;
        std::this_thread::sleep_for(std::chrono::milliseconds(backoffMs));

common/select.cpp:147

• The log-suppression state only tracks the last failing fd. If multiple selectables are failing and epoll returns different fds across iterations, s_consecutiveErrors is reset on each fd switch, so suppression may never reach kMaxConsecutiveErrorLogs and the process can still flood logs in a tight loop (especially when failures alternate across fds). This also doesn’t match the PR description’s “once per second” rate-limiting behavior.

Consider tracking suppression per-fd (e.g., thread-local map keyed by fd) and/or implementing a time-based rate limiter so alternating failures can’t bypass suppression.

            if (fd != s_lastFailedFd)
            {
                s_lastFailedFd = fd;
                s_consecutiveErrors = 0;
            }