-
Notifications
You must be signed in to change notification settings - Fork 19
Shbikram/de conflict tests #2207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ce814a8
e288b55
b7109f3
614c082
38c14d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. praise: just having random PK will largely reduce flaky test. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| package software.amazon.cryptography.examples; | ||
|
|
||
| import java.util.concurrent.ThreadLocalRandom; | ||
|
|
||
| /** | ||
| * Shared utilities for examples. The TEST_SUFFIX provides a unique suffix | ||
| * per JVM process so that parallel CI matrix jobs (different Java versions / OS) | ||
| * do not overwrite each other's items in the shared DynamoDB test table. | ||
| * | ||
| * Each example uses a hardcoded partition key like "rawEcdhKeyringItem". | ||
| * In CI, multiple JVMs run in parallel against the same table. Without | ||
| * isolation, one JVM can overwrite another's item, causing decrypt failures. | ||
| * By appending a random suffix, each JVM writes to its own partition key | ||
| * (e.g., "rawEcdhKeyringItem-482913") while keeping cross-example reads | ||
| * working within the same JVM. | ||
| */ | ||
| public class ExampleUtils { | ||
|
|
||
| private static final String TEST_SUFFIX = String.valueOf( | ||
| ThreadLocalRandom.current().nextInt(100000, 999999) | ||
| ); | ||
|
|
||
| /** | ||
| * Returns a partition key value unique to this JVM process. | ||
| * Example: "rawEcdhKeyringItem" becomes "rawEcdhKeyringItem-482913" | ||
| */ | ||
| public static String uniquePk(String base) { | ||
| return base + "-" + TEST_SUFFIX; | ||
| } | ||
| } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that partition key is random which will reduce flaky CI. This will end up creating huge DDB. So, we should put clean up code to remove the item created by examples. I would send the pk name from test then add clean up code in test to keep actual examples clean.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are multiple solutions to clean up.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me attempt a afterClass in each file. If that takes too long, we can park that and come back with another cleanup story. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to let it fail for visibility of the failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are we gaining from this visibility apart from having to retry? Is there any action item on failure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
RejectedExecutionExceptionexpected failure from this test?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. In parallel run two threads are scheduled to make call to DDB. If first one fails before other thread is even submitted, the executor shuts down and second submit fails leading to RejectedExecutionException.
Since this is a failure scenario, the first thread will always fail. In that case, it depends on whether the second thread is scheduled or not which is out of our control.