You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "dpgaspar (via GitHub)" <gi...@apache.org> on 2026/04/30 12:35:15 UTC

[PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

dpgaspar opened a new pull request, #39779:
URL: https://github.com/apache/superset/pull/39779

   ### SUMMARY
   
   When a Celery worker crashes (OOM, pod eviction) mid-execution, the report stays stuck in `WORKING` state in the database. On the next scheduled tick, `ReportWorkingState.next()` checks `is_on_working_timeout()`:
   
   **Before:** If the `working_timeout` has elapsed, it transitions to `ERROR` and raises `ReportScheduleWorkingTimeoutError`. For **daily schedules**, this means no retry until the next day — a 24-hour wait for something that could recover immediately.
   
   **After:** It transitions to `NOOP` instead and returns (no exception). The next cron tick picks it up naturally and retries. This:
   
   - **Fixes daily schedules**: They recover on the next tick instead of waiting 24 hours
   - **Avoids duplicate execution risk**: No immediate retry that could clash with celery's broker requeue (~30 min)
   - **No new config**: Uses the existing `working_timeout` per-report setting — no additional threshold to coordinate
   - **Single behavior**: One config, one meaning — "how long before a WORKING state is considered stuck"
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A — backend behavior change only.
   
   ### TESTING INSTRUCTIONS
   
   1. Create a report schedule with a `working_timeout` of e.g. 60 seconds
   2. Manually set the report's `last_state` to `WORKING` in the DB and create a `ReportExecutionLog` entry in `WORKING` state with `end_dttm` older than `working_timeout`
   3. Trigger the scheduler
   4. **Before**: Report transitions to `ERROR`, raises exception, daily schedules wait until next day
   5. **After**: Report transitions to `NOOP`, next cron tick retries it
   
   Unit tests updated to validate the new behavior.
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: SC-104379
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #39779:
URL: https://github.com/apache/superset/pull/39779#discussion_r3182408535


##########
superset/commands/report/execute.py:
##########
@@ -947,17 +961,26 @@ def next(self) -> None:
                 if last_working
                 else None
             )
-            logger.error(
-                "Working state timeout after %.2fs - execution_id: %s",
+            logger.warning(
+                "Working state timeout after %.2fs, resetting to NOOP "
+                "and retrying immediately - execution_id: %s",
                 elapsed_seconds if elapsed_seconds else 0,
                 self._execution_id,
             )
-            exception_timeout = ReportScheduleWorkingTimeoutError()
             self.update_report_schedule_and_log(
-                ReportState.ERROR,
-                error_message=str(exception_timeout),
+                ReportState.NOOP,

Review Comment:
   By setting as NOOP and recovering the stuck state, would we be introducing an infinite loop? Asking because I see no "stop" mechanism for the attempts after automatically re-scheduling X amount of time?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-5546400358

   @dpgaspar checking back in, this is still worth landing. @aminghadersohi's three review findings (the weakened integration-test assertion, the now-dead exception class, the persistent-OOM docstring note) are still open, plus it needs a rebase. Holler if you want me/us to push any commits here :D 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-4357755754

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-4352487642

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-4352563584

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/39779?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 63.59%. Comparing base ([`e4fe08a`](https://app.codecov.io/gh/apache/superset/commit/e4fe08ab9effe8c877783288c94e8d07768f999f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`856735d`](https://app.codecov.io/gh/apache/superset/commit/856735da395d2af34831af5546ae0dea127f30a1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #39779      +/-   ##
   ==========================================
   - Coverage   64.39%   63.59%   -0.81%     
   ==========================================
     Files        2567     2567              
     Lines      134392   134393       +1     
     Branches    31198    31199       +1     
   ==========================================
   - Hits        86548    85470    -1078     
   - Misses      46347    47408    +1061     
   - Partials     1497     1515      +18     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.70% <0.00%> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.46% <0.00%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.86% <100.00%> (-1.79%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.72% <100.00%> (+<0.01%)` | :arrow_up: |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/39779/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/39779?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-4352815763

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | b96ee04a40067f21de390f29e400dc2ece2162f0 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69f35753bdf906000840e487 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-39779--superset-docs-preview.netlify.app](https://deploy-preview-39779--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTM5Nzc5LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.1eZ9Bk-dCnGdMuOIpKOwPA-kQOB70FH7cexrG6VFmLk)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcxLDsIgEADQu8zaSTHQUryBx-AztI0IZKAxjfHusnur94XK5V07PABu4Nhmvw8Hmw9Km23V8tQ83oWS2kw2EXdsfqdwJsJcOn4Kv4684WgqcbowFsbB3K1LNM5ANZXrGca6mChnPUsXohGLEGJVgtSq4fcH) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-39779--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-4372041436

   <!-- Bito Reply -->
   The change resets stuck reports to NOOP and retries immediately via ReportNotTriggeredErrorState, which could lead to repeated retries if the report execution keeps getting stuck after each timeout. However, the design assumes this is safe post-celery requeue attempts, preventing infinite loops in normal scenarios.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(alerts): reset stuck WORKING reports to NOOP instead of ERROR on timeout [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39779:
URL: https://github.com/apache/superset/pull/39779#issuecomment-5086757741

   @dpgaspar this stuck-WORKING-report fix is worth landing, but it's currently conflicting and still has aminghadersohi's change requests open. Want to rebase and pick it back up? Wand me to push some commits here?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org