Skip to content

TCXB8-4164: Re-enable profile scheduler when hash-match skips dead thread - #410

Open
tabbas651 wants to merge 2 commits into
support/1.8from
topic/TCXB8-4164
Open

TCXB8-4164: Re-enable profile scheduler when hash-match skips dead thread#410
tabbas651 wants to merge 2 commits into
support/1.8from
topic/TCXB8-4164

Conversation

@tabbas651

Copy link
Copy Markdown
Contributor

When a profile's TimeoutThread dies silently (e.g. pthread init failure at boot during GFO restore), subsequent WebConfig or XConf pushes with the same hash skip re-registration, leaving the profile permanently unscheduled.

Add isProfileSchedulerRunning() to check if a profile's scheduler thread is alive. In both msgpack and JSON hash-match paths, call enableProfile() to restart the scheduler when the thread is not running.

…read

When a profile's TimeoutThread dies silently (e.g. pthread init failure
at boot during GFO restore), subsequent WebConfig or XConf pushes with
the same hash skip re-registration, leaving the profile permanently
unscheduled.

Add isProfileSchedulerRunning() to check if a profile's scheduler thread
is alive. In both msgpack and JSON hash-match paths, call enableProfile()
to restart the scheduler when the thread is not running.
Copilot AI lite review requested due to automatic review settings August 7, 2026 23:23
@tabbas651
tabbas651 requested a review from a team as a code owner August 7, 2026 23:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to prevent report profiles from becoming permanently unscheduled when their scheduler (TimeoutThread) dies silently and subsequent config pushes are skipped due to hash-match optimization. It introduces a scheduler-liveness check and attempts to re-enable profiles when the config hash hasn’t changed but scheduling is no longer active.

Changes:

  • Added isProfileSchedulerRunning(const char*) to the scheduler public API.
  • Implemented the liveness check in scheduler.c.
  • Updated both JSON and msgpack hash-match paths in reportprofiles.c to re-enable the profile if the scheduler is not running.

Reviewed changes

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

File Description
source/scheduler/scheduler.h Exposes new scheduler liveness API.
source/scheduler/scheduler.c Implements isProfileSchedulerRunning() to detect scheduler status per profile.
source/bulkdata/reportprofiles.c On hash-match, attempts to re-enable profiles when scheduler is not running (JSON + msgpack paths).
Suppressed comments (2)

source/bulkdata/reportprofiles.c:1000

  • Calling enableProfile(profileName) here is unlikely to restart a dead scheduler thread: enableProfile() returns early when profile->enable is already true, and registerProfileWithScheduler() also exits when a scheduler entry already exists for the profile name. If the existing scheduler entry’s TimeoutThread died silently, this path will still leave the profile unscheduled. A dedicated scheduler restart path (or making registerProfileWithScheduler recreate the thread when the existing one is not alive) seems necessary to meet the PR description.
                if(!isProfileSchedulerRunning(profileName))
                {
                    T2Warning("Profile %s scheduler not running, re-enabling\n", profileName);
                    enableProfile(profileName);
                }

source/bulkdata/reportprofiles.c:1415

  • Same as the JSON path above: enableProfile(profileName) will no-op when the profile is already enabled, and registerProfileWithScheduler() returns success when an entry already exists, so this is unlikely to restart a dead TimeoutThread. Consider adding a scheduler "restart" API or updating registerProfileWithScheduler() to recreate the thread when the existing one is not alive.
                if(!isProfileSchedulerRunning(profileName))
                {
                    T2Warning("Profile %s scheduler not running, re-enabling\n", profileName);
                    enableProfile(profileName);
                }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread source/scheduler/scheduler.c
Comment thread source/bulkdata/reportprofiles.c
Comment thread source/bulkdata/reportprofiles.c
Comment thread source/scheduler/scheduler.h
Copilot AI review requested due to automatic review settings August 7, 2026 23:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (4)

source/bulkdata/reportprofiles.c:1415

  • Same issue as the JSON path: enableProfile(profileName) will not restart scheduling if the profile is already enabled (enableProfile() returns early when profile->enable is true; see source/bulkdata/profile.c:1039-1044). This means the intended recovery for ‘hash match but dead scheduler thread’ will not occur.
                if(!isProfileSchedulerRunning(profileName))
                {
                    T2Warning("Profile %s scheduler not running, re-enabling\n", profileName);
                    enableProfile(profileName);
                }

source/bulkdata/reportprofiles.c:1000

  • The new ‘hash matches but scheduler thread is not running’ recovery path is not covered by unit tests. There is existing bulkdata unit test coverage for ReportProfiles_ProcessReportProfilesBlob / msgpack processing (source/test/bulkdata/reportprofilesTest.cpp), but no test exercises the hash-match branch with isProfileSchedulerRunning() returning false and verifies the expected recovery action is taken.
                if(!isProfileSchedulerRunning(profileName))
                {
                    T2Warning("Profile %s scheduler not running, re-enabling\n", profileName);
                    enableProfile(profileName);
                }

source/bulkdata/reportprofiles.c:1000

  • Calling enableProfile(profileName) here will not restart a dead scheduler thread when the Profile is already marked enabled. enableProfile() returns early for enabled profiles (source/bulkdata/profile.c:1039-1044), so this new recovery path is effectively a no-op in the failure scenario described in the PR (thread init failure but profile stays enabled). Consider adding an explicit ‘restart scheduler’ path (e.g., force re-register with scheduler when enabled but scheduler not running) instead of calling enableProfile unconditionally.

This issue also appears in the following locations of the same file:

  • line 996
  • line 1411
                if(!isProfileSchedulerRunning(profileName))
                {
                    T2Warning("Profile %s scheduler not running, re-enabling\n", profileName);
                    enableProfile(profileName);
                }

source/scheduler/scheduler.c:453

  • isProfileSchedulerRunning() uses pthread_kill(tProfile->tId, 0) to test liveness, but scheduler.c currently ignores pthread_create() failures when setting tProfile->tId (registerProfileWithScheduler() calls pthread_create without checking its return). In the PR’s described failure mode (thread creation/init failure), tProfile->tId may be left uninitialized/invalid, and calling pthread_kill on an invalid pthread_t is undefined on some pthread implementations. Consider recording thread-creation success (or only storing profiles in profileList after a successful pthread_create) so this liveness check is safe and reliable.
bool isProfileSchedulerRunning(const char* profileName)
{
    if(!sc_initialized || profileName == NULL || profileList == NULL)
        return false;

    if(pthread_mutex_lock(&scMutex) != 0)
        return false;

    size_t index = 0;
    for(; index < profileList->count; ++index)
    {
        SchedulerProfile *tProfile = (SchedulerProfile *)Vector_At(profileList, index);
        if(tProfile == NULL || tProfile->name == NULL)
            continue;
        if(strcmp(tProfile->name, profileName) == 0)
        {
            /* pthread_kill with signal 0 checks if the thread is still alive */
            bool running = (pthread_kill(tProfile->tId, 0) == 0);
            pthread_mutex_unlock(&scMutex);
            return running;

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.

2 participants