fix: prevent creating duplicate machine ids #116
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We saw the same machine id created more than one time. This caused an issue where the cli would not be able to connect to the correct machine. We now wait for the instance to be created. We don't care about what state the instance is in only that it has been handled by AWS. We believe duplication would happen when the scheduleTask would finish quickly, receive the next update from the API, but without AWS reporting the machine as pending. Co-authored-by: Billy Batista <[email protected]> Signed-off-by: Chris Goller <[email protected]>
|
Fixes DEP-1943 |
src/utils/aws.ts
Outdated
|
|
||
| // We are waiting for the instance to exist because we are blocking scheduleTask | ||
| // from starting another task with the same machine ID until this one completes. | ||
| while (!instanceCreated) { |
There was a problem hiding this comment.
I think this needs a sleep otherwise this is a tight loop of API requests
Possibly better and API aware, the AWS SDK should have a wait for method I believe, similar to the Go SDK https://aws.amazon.com/blogs/developer/waiters-in-modular-aws-sdk-for-javascript/
There was a problem hiding this comment.
I think this is the one, something like https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-ec2/Variable/waitForInstanceExists/:
const maxWaitTime = 30 // seconds
await waitForInstanceExists({client, maxWaitTime}, {InstanceIds: [instanceID]})Signed-off-by: Chris Goller <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We saw the same machine id created more than one time. This caused an issue where the cli would not be able to connect to the correct machine.
We now wait for the instance to be created.
We don't care about what state the instance is in
only that it has been handled by AWS.
We believe duplication would happen when the scheduleTask would finish quickly, receive the next update from the API, but without AWS reporting the machine as pending.
Note: another way we could fix this is to use the cache of changes as we did with volumes.