Skip to content

Upload size limits (GSI-2298)#166

Open
TheByronHimes wants to merge 14 commits intomainfrom
feature/upload_size_limits_GSI-2298
Open

Upload size limits (GSI-2298)#166
TheByronHimes wants to merge 14 commits intomainfrom
feature/upload_size_limits_GSI-2298

Conversation

@TheByronHimes
Copy link
Copy Markdown
Member

No description provided.

@TheByronHimes TheByronHimes requested review from Cito and mephenor April 16, 2026 07:06
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md Outdated

# Presigned URL rate limiting (optional)
max_part_urls_per_minute: int = 0 # 0 = disabled; token bucket refill rate
part_url_burst_size: int = 10 # Token bucket initial/max tokens
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would go for a slightly different name here to make clear what that means in isolation, i.e. how many presigned URLs per timespan.
Might also be worth to expose the time span separately instead of hardcoding it to a minute to better fine tune spacing of requests.

One minute might be a long enough time span to exhaust the token bucket at the very beginning and then wait for the rest.
Would be better if we can test and fine tune with different bucket sizes/time spans to condition that behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've adjusted it and think it fits better now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still not fully sold on that.

What about instead separating the time span an refill_rate per time span, so they are independently configurable and can be adapted to changing circumstances?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Per our offline discussion, we're using a refill interval param

Comment thread 88-pistol-shrimp/technical_specification.md
@TheByronHimes TheByronHimes requested a review from mephenor April 21, 2026 09:36
Comment thread 88-pistol-shrimp/technical_specification.md
@TheByronHimes TheByronHimes requested a review from mephenor April 24, 2026 13:57
mephenor
mephenor previously approved these changes Apr 28, 2026
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
Comment thread 88-pistol-shrimp/technical_specification.md Outdated
@TheByronHimes TheByronHimes requested a review from Cito April 29, 2026 14:23
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.

3 participants