Skip to content

Team query timeout setting #1072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Aug 30, 2025
Merged

Team query timeout setting #1072

merged 10 commits into from
Aug 30, 2025

Conversation

@SpencerTorres
Copy link
Contributor

@SpencerTorres SpencerTorres commented Aug 15, 2025

Closes HDX-2023

Behavior

Adds queryTimeout to the team settings page. If the setting is greater than 0, a max_execution_time setting is added to all queries executed by the ClickhouseClient. The timeout is in seconds, with 60 seconds being the default. 0 would be unlimited.

Images

Settings page:
settings page

Network log:
network log

@changeset-bot
Copy link

changeset-bot bot commented Aug 15, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: be1620b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/common-utils Minor
@hyperdx/api Minor
@hyperdx/app Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Aug 15, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Aug 29, 2025 9:12pm

@SpencerTorres
Copy link
Contributor Author

SpencerTorres commented Aug 15, 2025

Bit difficult to trace all usages of the client, let me know if I wrote anything crazy here 👀

@github-actions
Copy link
Contributor

github-actions bot commented Aug 15, 2025 •
edited
Loading

Stably Runner - Test Suite - 'Smoke Test'

Test Suite Run Result: 🟢 Success (4/4 tests passed) [dashboard]

This comment was generated from stably-runner-action

@teeohhem teeohhem self-requested a review August 15, 2025 15:36
teeohhem
teeohhem reviewed Aug 15, 2025
View reviewed changes
Copy link
Contributor

@teeohhem teeohhem left a comment

Choose a reason for hiding this comment

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

Great work!

I think there is one more place:
metadata.ts

There are also a few instances of queries getting run before the timeout setting is fetched, but we can investigate/punt on that until later.

Image

@SpencerTorres
Copy link
Contributor Author

SpencerTorres commented Aug 28, 2025

Perhaps those queries without the timeout were the metadata queries? I added the timeout to the metadata queries, it looks like those are cached and simply fetch the tables, so I kept the 60s default on those. Not sure if we care to apply the team configured timeout to those, but now they have some timeout in case they run long (I don't expect they will)

@teeohhem teeohhem self-requested a review August 29, 2025 20:18
teeohhem
teeohhem previously approved these changes Aug 29, 2025
View reviewed changes
Copy link
Contributor

@teeohhem teeohhem left a comment

Choose a reason for hiding this comment

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

LGTM!

@teeohhem teeohhem self-requested a review August 29, 2025 20:59
@kodiakhq kodiakhq bot merged commit 25f77aa into main Aug 30, 2025
16 of 18 checks passed
@kodiakhq kodiakhq bot deleted the spencer/feat_team_query_timeout branch August 30, 2025 02:42
brandon-pereira added a commit that referenced this pull request Nov 28, 2025
@brandon-pereira
Ensure correct query timeout value is used. For example, it defaults …
75ce009 
…to 60 seconds but a timeout gets created for 60 milliseconds.

Original PR:
#1072

Bug Introduced here:
#1132

Closes #1416
Fixes HDX-2931
@brandon-pereira brandon-pereira mentioned this pull request Nov 28, 2025
Merged
kodiakhq bot pushed a commit that referenced this pull request Dec 2, 2025
@brandon-pereira
Ensure correct query timeout value is used. (#1417)
3f94105 
For example, it defaults to 60 seconds, but a timeout gets created for 60 milliseconds.

## Before

<img width="2850" height="1264" alt="Screenshot 2025-11-28 at 2 11 33 PM" src="https://github.com/user-attachments/assets/8eff105b-39b0-459b-bdf3-a149645e3c57" />

## After

<img width="1848" height="1086" alt="Screenshot 2025-11-28 at 2 19 20 PM" src="https://github.com/user-attachments/assets/406f53ed-cda0-4ccc-9442-249d6a507a5a" />

Original PR:
#1072

Bug Introduced here:
#1132

Closes #1416
Fixes HDX-2931
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teeohhem teeohhem teeohhem approved these changes

Assignees

No one assigned

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SpencerTorres @teeohhem