mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 13:41:31 +08:00
FIX: Use hidden site setting for batch presign rate limit (#16692)
This was causing issues on some sites, having the const, because this really is heavily dependent on upload speed. We request 5-10 URLs at a time with this endpoint; for a 1.5GB upload with 5mb parts this could mean 60 requests to the server to get all the part URLs. If the user's upload speed is super fast they may request all 60 batches in a minute, if it is slow they may request 5 batches in a minute. The other external upload endpoints are not hit as often, so they can stay as constant values for now. This commit also increases the default to 20 requests/minute.
This commit is contained in:
parent
fbcc35b417
commit
244836ddd4
|
@ -1886,6 +1886,9 @@ rate_limits:
|
||||||
max_allowed_secondary_emails:
|
max_allowed_secondary_emails:
|
||||||
default: 10
|
default: 10
|
||||||
hidden: true
|
hidden: true
|
||||||
|
max_batch_presign_multipart_per_minute:
|
||||||
|
default: 20
|
||||||
|
hidden: true
|
||||||
|
|
||||||
developer:
|
developer:
|
||||||
force_hostname:
|
force_hostname:
|
||||||
|
|
|
@ -10,7 +10,6 @@ module ExternalUploadHelpers
|
||||||
PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE = 10
|
PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE = 10
|
||||||
CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10
|
CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10
|
||||||
COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10
|
COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10
|
||||||
BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE = 10
|
|
||||||
|
|
||||||
included do
|
included do
|
||||||
before_action :external_store_check, only: [
|
before_action :external_store_check, only: [
|
||||||
|
@ -112,8 +111,17 @@ module ExternalUploadHelpers
|
||||||
part_numbers = params.require(:part_numbers)
|
part_numbers = params.require(:part_numbers)
|
||||||
unique_identifier = params.require(:unique_identifier)
|
unique_identifier = params.require(:unique_identifier)
|
||||||
|
|
||||||
|
##
|
||||||
|
# NOTE: This is configurable by hidden site setting because this really is heavily
|
||||||
|
# dependent on upload speed. We request 5-10 URLs at a time with this endpoint; for
|
||||||
|
# a 1.5GB upload with 5mb parts this could mean 60 requests to the server to get all
|
||||||
|
# the part URLs. If the user's upload speed is super fast they may request all 60
|
||||||
|
# batches in a minute, if it is slow they may request 5 batches in a minute.
|
||||||
|
#
|
||||||
|
# The other external upload endpoints are not hit as often, so they can stay as constant
|
||||||
|
# values for now.
|
||||||
RateLimiter.new(
|
RateLimiter.new(
|
||||||
current_user, "batch-presign", ExternalUploadHelpers::BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE, 1.minute
|
current_user, "batch-presign", SiteSetting.max_batch_presign_multipart_per_minute, 1.minute
|
||||||
).performed!
|
).performed!
|
||||||
|
|
||||||
part_numbers = part_numbers.map do |part_number|
|
part_numbers = part_numbers.map do |part_number|
|
||||||
|
|
|
@ -1056,23 +1056,22 @@ describe UploadsController do
|
||||||
it "rate limits" do
|
it "rate limits" do
|
||||||
RateLimiter.enable
|
RateLimiter.enable
|
||||||
RateLimiter.clear_all!
|
RateLimiter.clear_all!
|
||||||
|
SiteSetting.max_batch_presign_multipart_per_minute = 1
|
||||||
|
|
||||||
stub_const(ExternalUploadHelpers, "BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE", 1) do
|
stub_list_multipart_request
|
||||||
stub_list_multipart_request
|
post "/uploads/batch-presign-multipart-parts.json", params: {
|
||||||
post "/uploads/batch-presign-multipart-parts.json", params: {
|
unique_identifier: external_upload_stub.unique_identifier,
|
||||||
unique_identifier: external_upload_stub.unique_identifier,
|
part_numbers: [1, 2, 3]
|
||||||
part_numbers: [1, 2, 3]
|
}
|
||||||
}
|
|
||||||
|
|
||||||
expect(response.status).to eq(200)
|
expect(response.status).to eq(200)
|
||||||
|
|
||||||
post "/uploads/batch-presign-multipart-parts.json", params: {
|
post "/uploads/batch-presign-multipart-parts.json", params: {
|
||||||
unique_identifier: external_upload_stub.unique_identifier,
|
unique_identifier: external_upload_stub.unique_identifier,
|
||||||
part_numbers: [1, 2, 3]
|
part_numbers: [1, 2, 3]
|
||||||
}
|
}
|
||||||
|
|
||||||
expect(response.status).to eq(429)
|
expect(response.status).to eq(429)
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue
Block a user