Skip to content

Commit 143c811

Browse files
Release 0.4.0
- Upgrade awscr-s3 (connection pool for the S3 client, SSL Context reuse) fixes #4
1 parent 6cce46b commit 143c811

File tree

3 files changed

+141
-5
lines changed

3 files changed

+141
-5
lines changed

shard.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: easy-awscr
2-
version: 0.3.3
2+
version: 0.4.0
33

44
authors:
55
- Philipp Claßen <philipp.classen@posteo.com>
@@ -12,6 +12,6 @@ dependencies:
1212
commit: 8b1853609c069a1e70b967a7d654e07a0dbdbcfc
1313
awscr-s3:
1414
github: taylorfinnell/awscr-s3
15-
commit: 47c97e2f545ed5fead268a02b58501e0c968ec65
15+
commit: 2e93fb9e8c74f11a24c5808fc4b1f8b304181830
1616

1717
license: MIT

src/easy-awscr/s3/client.cr

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,34 @@
11
require "awscr-s3"
2+
require "./internals/connection_pool"
23
require "./internals/async_chunk_uploader"
34

45
module EasyAwscr::S3
56
class Client
67
@s3_client : Awscr::S3::Client?
8+
@client_factory : Internals::ConnectionPool?
79

810
# This is a hard limit enforced by AWS for multipart uploads:
911
# each part must be at least 5 MB.
1012
MINIMUM_PART_SIZE_5MB = 5242880
1113

14+
# From time to time, we should recreated the connection pool, so it will reload
15+
# the SSL context (see https://github.com/crystal-lang/crystal/issues/15419).
16+
DEFAULT_POOL_REFRESH_INTERVAL = 24.hours
17+
1218
def initialize(*,
1319
@region = EasyAwscr::Config.default_region!,
1420
@credential_provider = EasyAwscr::Config.default_credential_provider,
1521
lazy_init = false)
16-
@mutex = Mutex.new
22+
@mutex = Mutex.new(:unchecked)
1723
client unless lazy_init
1824
end
1925

26+
private def create_connection_pool
27+
# TODO: this uses the defaults, but it would make sense to
28+
# let the user overwrite them.
29+
Internals::ConnectionPool.new
30+
end
31+
2032
# List s3 buckets
2133
#
2234
# ```
@@ -265,15 +277,32 @@ module EasyAwscr::S3
265277
end
266278

267279
private def client(*, force_new = false) : Awscr::S3::Client
280+
dead_client_factory = nil
268281
@mutex.synchronize do
269282
s3_client = @s3_client
270-
if s3_client && !force_new
283+
if s3_client && !force_new && !client_factory_needs_refresh?
271284
s3_client
272285
else
273286
cred = @credential_provider.credentials
274-
@s3_client = Awscr::S3::Client.new(@region, cred.access_key_id, cred.secret_access_key, cred.session_token)
287+
288+
# refresh the connection pool (updates also the SSL context)
289+
client_factory = create_connection_pool
290+
dead_client_factory = @client_factory
291+
@client_factory = client_factory
292+
293+
@s3_client = Awscr::S3::Client.new(@region, cred.access_key_id, cred.secret_access_key, cred.session_token, client_factory: client_factory)
275294
end
276295
end
296+
ensure
297+
dead_client_factory.try &.close
298+
end
299+
300+
private def client_factory_needs_refresh? : Bool
301+
if cf = @client_factory
302+
Time.utc - cf.created_at > DEFAULT_POOL_REFRESH_INTERVAL
303+
else
304+
false
305+
end
277306
end
278307
end
279308
end
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
require "http/client"
2+
3+
module EasyAwscr::S3::Internals
4+
class ConnectionPool < Awscr::S3::HttpClientFactory
5+
getter created_at
6+
7+
def initialize(*, @max_ttl : Time::Span? = 5.minutes, @max_size = 128)
8+
@pool = Hash(Fiber, {HTTP::Client, Time}).new
9+
@tls = OpenSSL::SSL::Context::Client.new
10+
@mutex = Mutex.new(:unchecked)
11+
@closed = false
12+
@created_at = Time.utc
13+
end
14+
15+
def acquire_client(endpoint : URI, signer : Awscr::Signer::Signers::Interface) : HTTP::Client
16+
@mutex.synchronize { @pool.delete(Fiber.current) }.try do |client, last_checked|
17+
if expired?(last_checked)
18+
client.close
19+
else
20+
return client
21+
end
22+
end
23+
24+
# creates a new client
25+
super
26+
end
27+
28+
# Overwritten only to call "reset_headers" (see comment there for details).
29+
#
30+
# Note: At this point it is not clear if the workaround can be improved. Should
31+
# it become clear that it cannot, then this code should perhaps be moved into the
32+
# awscr-s3 library (since then every user would need to replicate the code here).
33+
protected def attach_signer(client, signer)
34+
if signer.is_a?(Awscr::Signer::Signers::V4)
35+
client.before_request do |req|
36+
reset_headers(req)
37+
signer.as(Awscr::Signer::Signers::V4).sign(req, encode_path: false)
38+
end
39+
else
40+
client.before_request do |req|
41+
reset_headers(req)
42+
signer.sign(req)
43+
end
44+
end
45+
end
46+
47+
# Workaround to avoid signing errors when requests have to be repeated
48+
# after a TCPSocket has to be reconnected.
49+
#
50+
# Background:
51+
# * https://github.com/taylorfinnell/awscr-signer/issues/56
52+
# * https://github.com/crystal-lang/crystal/issues/16028
53+
private def reset_headers(req)
54+
req.headers.delete "Authorization"
55+
req.headers.delete "X-Amz-Content-Sha256"
56+
req.headers.delete "X-Amz-Date"
57+
end
58+
59+
private def expired?(last_checked, now = Time.utc)
60+
@max_ttl.try { |ttl| now - last_checked > ttl }
61+
end
62+
63+
def acquire_raw_client(endpoint : URI) : HTTP::Client
64+
HTTP::Client.new(endpoint, tls: @tls)
65+
end
66+
67+
def release(client : HTTP::Client?)
68+
return unless client
69+
70+
if @max_size == 0
71+
client.close
72+
return
73+
end
74+
75+
now = Time.utc
76+
dead1 = nil
77+
dead2 = nil
78+
dead3 = nil
79+
80+
current_fiber = Fiber.current
81+
@mutex.synchronize do
82+
unless @closed
83+
@pool.first_key?.try do |fiber|
84+
dead1 = @pool.shift[1][0] if fiber.dead? || expired?(@pool.first_value[1], now)
85+
@pool.delete(current_fiber).try { |old_client, _| dead2 = old_client }
86+
end
87+
@pool[current_fiber] = {client, now}
88+
dead3 = @pool.shift[1][0] if @pool.size > @max_size
89+
end
90+
end
91+
ensure
92+
dead1.try &.close
93+
dead2.try &.close
94+
dead3.try &.close
95+
end
96+
97+
def close
98+
@mutex.synchronize do
99+
return if @closed
100+
101+
@closed = true
102+
@pool.values.each { |client, _| client.close }
103+
@pool.clear
104+
end
105+
end
106+
end
107+
end

0 commit comments

Comments
 (0)