Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type" : "bugfix",
"description" : "Amazon Q Code Transformation: retry project upload up to 2 times"
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import java.util.Base64
import java.util.concurrent.CancellationException
import java.util.concurrent.atomic.AtomicBoolean
import javax.net.ssl.SSLHandshakeException
import kotlin.math.pow

const val MAX_ZIP_SIZE = 2000000000 // 2GB
const val EXPLAINABILITY_V1 = "EXPLAINABILITY_V1"
Expand Down Expand Up @@ -394,17 +395,24 @@ class CodeModernizerSession(
throw AlreadyDisposedException("Disposed when about to upload project artifact to s3")
}
val uploadStartTime = Instant.now()
try {
clientAdaptor.uploadArtifactToS3(
createUploadUrlResponse.uploadUrl(),
payload,
sha256checksum,
createUploadUrlResponse.kmsKeyArn().orEmpty(),
) { shouldStop.get() }
} catch (e: Exception) {
LOG.error { "Unexpected error when uploading project artifact to S3: $e" }
throw e // pass along error to callee
for (i in 0..2) {
try {
clientAdaptor.uploadArtifactToS3(
createUploadUrlResponse.uploadUrl(),
payload,
sha256checksum,
createUploadUrlResponse.kmsKeyArn().orEmpty(),
) { shouldStop.get() }
break
} catch (e: Exception) {
LOG.error { "Unexpected error when uploading project artifact to S3 on attempt ${i+1}/3: ${e.localizedMessage}" }
if (i == 2) {
throw e
}
Thread.sleep((1000 * 2.0.pow(i)).toLong())
}
}
LOG.info { "Upload to S3 succeeded" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of implementing our own retry logic rather than doing it at the SDK layer, but the existing S3 client's upload functionality requires a bucket name as a part of the request, and 1) I don't think we should have the internal S3 bucket names we use floating around this repo, 2) we use multiple different buckets, and 3) we don't have access to the bucket name in the response of our CreateUploadUrl — this API would have to be changed and then we can use the existing S3 client.

Copy link
Contributor Author

@dhasani23 dhasani23 Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: use waitUntil instead of a for loop; makes this retry logic even simpler

if (!shouldStop.get()) {
LOG.info { "Uploaded artifact. Latency: ${calculateTotalLatency(uploadStartTime, Instant.now())}ms" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class GumbyClient(private val project: Project) {
var result: CodeWhispererRuntimeResponse? = null
try {
result = apiCall()
LOG.info { "$apiName request ID: ${result.responseMetadata().requestId()}" }
return result
} catch (e: Exception) {
LOG.error(e) { "$apiName failed: ${e.message}" }
Expand Down
Loading