Skip to content

Commit 33e41fb

Browse files
vdegovethbar
andauthored
Upload S3 multipart : envoi d’erreurs sur Sentry, tweak timeout et concurrency pour la consolidation IRVE (#5551)
Co-authored-by: Thibaut Barrère <thibaut.barrere@gmail.com>
1 parent 3c1012d commit 33e41fb

4 files changed

Lines changed: 49 additions & 13 deletions

File tree

apps/shared/lib/s3.ex

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,34 @@ defmodule Transport.S3 do
3535
bucket |> ExAws.S3.delete_object(path) |> Transport.Wrapper.ExAWS.impl().request!()
3636
end
3737

38-
@spec stream_to_s3!(bucket_feature(), binary(), binary(), acl: atom(), cache_control: binary()) :: any()
38+
@spec stream_to_s3!(bucket_feature(), binary(), binary(),
39+
acl: atom(),
40+
cache_control: binary(),
41+
timeout: pos_integer(),
42+
max_concurrency: pos_integer()
43+
) :: any()
3944
def stream_to_s3!(feature, local_path, upload_path, options \\ []) do
4045
Logger.debug("Streaming #{local_path} to #{upload_path}")
41-
options = Keyword.validate!(options, [:cache_control, {:acl, :private}])
46+
options = Keyword.validate!(options, [:cache_control, :timeout, :max_concurrency, {:acl, :private}])
4247

4348
local_path
4449
|> ExAws.S3.Upload.stream_file()
4550
|> ExAws.S3.upload(Transport.S3.bucket_name(feature), upload_path, options)
4651
|> Transport.Wrapper.ExAWS.impl().request!()
52+
catch
53+
# ExAws uploads multipart parts via `Task.async_stream`; a large/slow upload can hit its
54+
# timeout, which surfaces as a process *exit* (not an exception, so it escapes `rescue` and
55+
# `Sentry.capture_exception`). Report it for observability, then let it propagate unchanged.
56+
# NOTE: relies on current internal behaviour of ExAws here:
57+
# https://github.com/ex-aws/ex_aws_s3/blob/v2.5.9/lib/ex_aws/s3/upload.ex#L133-L141
58+
:exit, reason ->
59+
Sentry.capture_message("S3 upload failed (exit), likely a timeout",
60+
level: :error,
61+
result: :sync,
62+
extra: %{feature: inspect(feature), upload_path: upload_path, reason: inspect(reason)}
63+
)
64+
65+
exit(reason)
4766
end
4867

4968
@spec download_file(bucket_feature(), binary(), binary()) :: any()

apps/shared/test/support/s3_test_utils.ex

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,12 @@ defmodule Transport.Test.S3TestUtils do
4343
src: src = %File.Stream{},
4444
bucket: ^expected_bucket,
4545
path: path,
46-
opts: [acl: ^expected_acl],
46+
opts: opts,
4747
service: :s3
4848
} ->
49+
# match `acl` within the opts rather than the whole list, so callers may pass extra
50+
# options (e.g. `timeout:`) without breaking this expectation
51+
assert Keyword.get(opts, :acl) == expected_acl
4952
assert String.starts_with?(path, expected_start_path)
5053
assert src |> Enum.join("\n") == expected_file_content
5154
end)

apps/transport/lib/S3/aggregates_uploader.ex

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,28 @@ defmodule Transport.S3.AggregatesUploader do
33
Helpers to upload a file, computes its sha256, and update a "latest" file.
44
"""
55

6-
@spec upload_aggregate!(Path.t(), String.t(), String.t()) :: :ok
6+
@spec upload_aggregate!(Path.t(), String.t(), String.t(), Keyword.t()) :: :ok
77
@doc """
88
This method takes a local `file` and upload 4 different files to our S3 `aggregates` bucket (the bucket is expected to exist):
99
- the `remote_path` and `remote_latest_path` containing the data from `file`
1010
- two companions files with `.sha256sum` extension appended (SHA256 sum is computed on the fly)
1111
12+
`options` are forwarded to `Transport.S3.stream_to_s3!/4`. Useful options:
13+
- `timeout:` it applies to each part of the multipart upload (and not the whole file!). Defaults to 30s, but may be increased for large files.
14+
- `max_concurrency:` it applies to the number of concurrent parts uploaded. Defaults to 4.
15+
1216
Example
1317
1418
with_tmp_file(fn file ->
1519
File.write(file, "some relevant data")
1620
upload_aggregate!(file, "aggregate-20250127193035.csv", "aggregate-latest.csv")
1721
end)
1822
"""
19-
def upload_aggregate!(file, remote_path, remote_latest_path) do
23+
def upload_aggregate!(file, remote_path, remote_latest_path, options \\ []) do
2024
with_tmp_file(fn checksum_file ->
2125
sha256!(file, checksum_file)
2226

23-
upload_files!(file, checksum_file, remote_path)
27+
upload_files!(file, checksum_file, remote_path, options)
2428
|> update_latest_files!(remote_latest_path)
2529
end)
2630
end
@@ -61,11 +65,11 @@ defmodule Transport.S3.AggregatesUploader do
6165
File.write!(checksum_file, hash)
6266
end
6367

64-
defp upload_files!(file, checksum_file, remote_path) do
68+
defp upload_files!(file, checksum_file, remote_path, options) do
6569
remote_checksum_path = checksum_filename(remote_path)
6670

67-
stream_upload!(file, remote_path)
68-
stream_upload!(checksum_file, remote_checksum_path)
71+
stream_upload!(file, remote_path, options)
72+
stream_upload!(checksum_file, remote_checksum_path, options)
6973

7074
{remote_path, remote_checksum_path}
7175
end
@@ -83,8 +87,8 @@ defmodule Transport.S3.AggregatesUploader do
8387
"#{base_filename}.sha256sum"
8488
end
8589

86-
defp stream_upload!(file, filename) do
87-
Transport.S3.stream_to_s3!(:aggregates, file, filename)
90+
defp stream_upload!(file, filename, options) do
91+
Transport.S3.stream_to_s3!(:aggregates, file, filename, options)
8892
end
8993

9094
defp copy!(s3_path, filename) do

apps/transport/lib/irve/consolidation.ex

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ defmodule Transport.IRVE.Consolidation do
1919
@consolidated_file_no_dedup_base_name "consolidation_transport_avec_doublons_irve_statique"
2020
@consolidated_file_base_name "consolidation_transport_irve_statique"
2121

22+
# The consolidated files can be large; uploading them can exceed ExAws' default 30s multipart
23+
# timeout. We lower the concurrency of the per-file parts (each part then gets more bandwidth
24+
# and uploads faster) and give each part a 1 minute ceiling.
25+
@s3_upload_timeout :timer.minutes(1)
26+
@s3_upload_max_concurrency 2
27+
2228
def process(opts \\ []) do
2329
destination = Keyword.get(opts, :destination, :send_to_s3)
2430
debug = Keyword.get(opts, :debug, false)
@@ -164,7 +170,9 @@ defmodule Transport.IRVE.Consolidation do
164170
upload_aggregate!(
165171
report_file,
166172
"#{@report_output_base_name}_#{timestamp()}.csv",
167-
"#{@report_output_base_name}.csv"
173+
"#{@report_output_base_name}.csv",
174+
timeout: @s3_upload_timeout,
175+
max_concurrency: @s3_upload_max_concurrency
168176
)
169177
end)
170178

@@ -186,7 +194,9 @@ defmodule Transport.IRVE.Consolidation do
186194
upload_aggregate!(
187195
consolidation_file,
188196
"#{base_name}_#{timestamp()}.csv",
189-
"#{base_name}.csv"
197+
"#{base_name}.csv",
198+
timeout: @s3_upload_timeout,
199+
max_concurrency: @s3_upload_max_concurrency
190200
)
191201
end)
192202
end

0 commit comments

Comments
 (0)