Skip to content

Remove HardLimitingWorker by @SchrosCat2013 fixing #6424 - #7194

Closed
pavelbrylov wants to merge 3 commits into
getredash:masterfrom
jobtoday:master
Closed

pavelbrylov wants to merge 3 commits into
getredash:masterfrom
jobtoday:master

Conversation

@pavelbrylov

@pavelbrylov pavelbrylov commented Oct 18, 2024

Copy link
Copy Markdown

What type of PR is this?

  • [X ] Bug Fix

Description

There is an issue with adhoc queries running longer than minute, there a lot of details in #6424 (comment)
Author of investigation and changes is @SchrosCat2013, I just adopted changes in his fork to the lates master in a hope of not having to maintain own fork :)

How is this tested?

  • [ X ] Manually

We patched and built own image, this solves the issue and same long running query is now polled until it completes as opposed to stop polling in 1 minute.

Related Tickets & Documents

#6424

@pavelbrylov

Copy link
Copy Markdown
Author

@justinclift if you could help prioritize this - would be great :) Thanks!

@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes Redash's custom HardLimitingWorker, delegating work-horse monitoring and timeout enforcement to RQ while retaining StatsD instrumentation and the custom cancellable job type.

  • Simplifies RedashWorker inheritance.
  • Removes custom timeout, heartbeat, process-exit, and cancellation monitoring.
  • Leaves running-query cancellation without the signal path needed to interrupt query execution.

Confidence Score: 4/5

This PR is not safe to merge until running-query cancellation still interrupts the active work horse.

The query cancellation endpoint now only updates job state; removing the custom monitor eliminates the path that observes that state and signals the process executing the database query.

Files Needing Attention: redash/tasks/worker.py

Important Files Changed

Filename Overview
redash/tasks/worker.py Removes the custom work-horse monitor and hard-limit implementation, but also drops the signal-based handling required by Redash's running-job cancellation flow.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[DELETE running query job] --> B[CancellableJob.cancel]
  B --> C[Store cancelled metadata]
  C --> D{Worker monitor}
  D -->|Before this PR| E[Refresh cancellation state]
  E --> F[Send SIGINT to work horse]
  F --> G[Query runner cancels database operation]
  D -->|After this PR| H[Inherited RQ monitoring]
  H --> I[No Redash cancellation signal]
  I --> J[Query process may continue running]
Loading

Reviews (1): Last reviewed commit: "Merge branch 'master' into master" | Re-trigger Greptile

Comment thread redash/tasks/worker.py


class RedashWorker(StatsdRecordingWorker, HardLimitingWorker):
class RedashWorker(StatsdRecordingWorker):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Running Cancellation No Longer Interrupts

Removing HardLimitingWorker also removes the monitor path that refreshed Redash's cancellation metadata and sent SIGINT to a running work horse. When the query-job DELETE endpoint calls CancellableJob.cancel(), the job is marked cancelled in Redis, but the executing query process is not interrupted. Its query-runner cancellation handler is therefore never invoked, and the query can continue consuming worker and database capacity.

Knowledge Base Used: Background jobs and scheduling

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.

1 participant