According to this discussion, the SDK clients are intended to be reused. We should create one client in main and then wrap it in an Arc, so that we reuse the underlying client struct when we clone.
Details
Details
Confirmed that I could make requests to DDB with the wrapped client
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- ENG-1351 (branched from master)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I assume you tested this code before without Arc? Do you know when exactly this is being reused, in other words, when exactly this Arc comes into play? And what would cause this code to fail if we don't use Arc?
Comment Actions
Having an Arc wrapper for the client is useful because we have to clone self.client in a couple spots to satisfy the borrow checker. The code probably wouldn’t fail even if we cloned the underlying client without Arc, but it’d be inefficient. This is really a “best practices” fix, not a bug fix
Comment Actions
The code probably wouldn’t fail even if we cloned the underlying client without Arc, but it’d be inefficient.
To clarify, I meant that even if we received a ton of traffic and cloned the client a bunch, it would probably still work. It definitely works as is under light traffic
Comment Actions
It is a good practice to use Arc::clone() instead of .clone() to distinguish between cloning data vs creating a new reference.
Comment Actions
the DynamoDB client in the SDK only holds an Arc<Handle> so we can clone freely already. Abandoning
Comment Actions
Relying on an implementation detail from a library sounds fragile. They may change this detail with any update, so we should write our code in a way that wouldn't be affected by this.