Skip to content
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.

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

Max values for settings #82

Open
steve-chavez opened this issue Feb 9, 2024 · 2 comments
Open

Max values for settings #82

steve-chavez opened this issue Feb 9, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Feb 9, 2024

Problem

Found an user doing the following for an API role:

alter role service_role set statement_timeout = '15min';

Which makes no sense because the API roles should are bounded by HTTP timeouts. Cloudflare for example enforces a 100 second timeout.

Proposal 1

Enforce max values for role settings. For this case we could do:

supautils.role_max_settings = '{"service_role": { "statement_timeout": { "max": "100s"}}}'

If the user surpasses the setting then we'd fail and show an error message.

Proposal 2

Not sure if minimum values would make sense. But if so maybe we could use a pg range to keep the config shorter:

supautils.role_settings_bounds = '{"statement_timeout": { "service_role": "[5,100)", "anon": "[5,30)" }'
@steve-chavez steve-chavez added the enhancement New feature or request label Feb 9, 2024
@soedirgo
Copy link
Member

soedirgo commented Feb 22, 2024

I think this is better handled on the backend/infra side similar to RDS, since we might want to make the limits dynamic wrt instance size etc.

Role settings might be a bit tricky though.

@steve-chavez
Copy link
Member Author

steve-chavez commented Feb 23, 2024

@soedirgo We already query resources for https://github.com/supabase/supautils?tab=readme-ov-file#constrained-extensions. We could do the same for this.

supautils.settings_bounds = '{"statement_timeout": { "max": { "mem": ["(100,1G)", "(200, 2G)"]} }'

Not sure if it would make sense for statement_timeout though but maybe for work_mem?

Also, it looks it would be another feature, so it can be done later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants