>From Hussain Towaileb <[email protected]>: Attention is currently required from: Michael Blow, Murtadha Hubail.
Hussain Towaileb has posted comments on this change by Hussain Towaileb. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493?usp=email ) Change subject: [ASTERIXDB-3659][EXT]: delegate assume role auth to AWS SDK ...................................................................... Patch Set 5: (5 comments) File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/ExternalProperties.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493/comment/4afd804c_0d0a9c23?usp=email : PS5, Line 63: AWS_ASSUME_ROLE_STALE_TIME( > `STALE_TIME` mean something different than `CREDENTIALS_TTL`? It is different than TTL in the sense stale means we need to update, but the credentials are still valid and usable. Example: If we assume the role for 1 hour, have a stale of 5 minutes, and pre-fetch 5 minutes, state is like this: 0-54 (credentials are valid + not stale) 55+ (credentials still valid + stale so it needs updating) Prefetch is how early we want to refresh relative to stale, so stale is 55, prefetch is 5, so we will try to refresh at 50+. This is all to try and refresh the credentials before the request expires. Note that we can fail to assume few times, but because we have a buffer, the SDK keeps retrying, and once it succeeds, it switches to use the new ones. I have set the properties of each value to the default it has in the AWS SDK, for now at least. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493/comment/924fe5b0_b849b539?usp=email : PS5, Line 64: POSITIVE_INTEGER, > isn't `0` a legal value, that it should always be refreshed? The credentials will always refresh regardless, just a matter of when to try to refresh them. On bad/unstable network, refreshing can take a while, hence those parameters. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493/comment/e4d720ce_1a86c11c?usp=email : PS5, Line 70: "Time in seconds before the AWS assumed role credentials are considered close to stale and should be" > I'm confused- if they're stale after 60s, how is 300s "close to stale"? See details of the example above. Although it also didn't make sense to me to have both parameters, since I can make stale 6 minutes, which is identical to 1 minute stale + prefetch together, and no need to prefetch at all. But since it has a default in the SDK, I made it configurable. It is possible that stale does not tell it to refresh, but rather sets the status as "stale" so other internal stuff can work with it as well, hence, one says stale, and one says this is when you start prefetching. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493/comment/e9b3244c_1f967ee1?usp=email : PS5, Line 74: false, > OOC- why do we default to false? I wanted to default this to true, but it defaults to false in the SDK, so I kept it like this for now. The overhead of this is 1 extra thread and a tiny memory. And it is nice because it starts refreshing on time, regardless of the system state. For example, I could be reaching the prefetch window, but I already downloaded a chunk and I am processing it (let's say it is a super complex chunk). We will not do the refresh unless the processing is done, and we hit the AWS service to get the next chunk, and refresh the credentials then. I tested it with debug points, I made it stale on 50, and only release the debug at 59, and only then refreshed. I prefer going async if we are fine with having an extra thread per task/partition since we work in parallel. File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/AwsUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493/comment/f05a2f7e_4db95721?usp=email : PS5, Line 335: } > for L324-335, should we use one of those helpers that ensures a failure > doesn't result in a leak? By leak, do you mean the spammy log? If so, you're right, we should. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20493?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: phoenix Gerrit-Change-Id: I6a3c755f94d377b443f21594443fac830875610e Gerrit-Change-Number: 20493 Gerrit-PatchSet: 5 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Attention: Murtadha Hubail <[email protected]> Gerrit-Attention: Michael Blow <[email protected]> Gerrit-Comment-Date: Thu, 23 Oct 2025 01:32:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michael Blow <[email protected]>
