Skip to content
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

dynamodb.TableExistsWaiter won't wait if waitTime < minDelay #2551

Closed
2 tasks done
mmunoz3 opened this issue Mar 13, 2024 · 3 comments
Closed
2 tasks done

dynamodb.TableExistsWaiter won't wait if waitTime < minDelay #2551

mmunoz3 opened this issue Mar 13, 2024 · 3 comments
Assignees
Labels
bug This issue is a bug.

Comments

@mmunoz3
Copy link

mmunoz3 commented Mar 13, 2024

Acknowledgements

Describe the bug

The table waiter won't wait at all when passed waitTime < MinDelay.

So to get any wait time smaller than the default 20 seconds stablished for MinDelay the caller is forced to set this MinDelay optional parameter to something smaller than the wanted waitTime:

Expected Behavior

The waiter should wait for waitTime without having to worry about the MinDelay optional parameter.

Current Behavior

The waiter performs the check :

if remainingTime < options.MinDelay || remainingTime <= 0 {
	break
}

Deciding it doesn't have to wait as if it were in the middle of the computational backoff and returning the error.

Reproduction Steps

	ctx := context.Background()

	conf, _ := config.LoadDefaultConfig(ctx)

	client := dynamodb.NewFromConfig(conf)

	waiter := dynamodb.NewTableExistsWaiter(client)

	waitTime := 10 * time.Second

	err := waiter.Wait(ctx, &dynamodb.DescribeTableInput{
		TableName: aws.String("myTable"),
	}, waitTime)

Possible Solution

Implement some logic like: "if this is the first wait and remainingTime < minDelay then wait remainingTime"

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2/service/dynamodb v1.27.1

Compiler and Version used

go version go1.21.5 darwin/arm64

Operating System and version

MacOS Sonoma

@mmunoz3 mmunoz3 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 13, 2024
@RanVaknin RanVaknin self-assigned this Mar 14, 2024
@RanVaknin RanVaknin removed the needs-triage This issue or PR still needs to be triaged. label Mar 15, 2024
@RanVaknin
Copy link
Contributor

Hi @mmunoz3 ,

Thanks for reaching out.

The behavior you are describing here is the expected behavior, and not a bug. The expectation that the wait operation should execute at least once regardless of the minDelay and maxWaitDur values is not the expected behavior. The SDK's waiter logic is designed to respect these parameters strictly. If your set value for (or service default for)minDelay is longer than the maxWaitDur, the waiter concludes that there is not enough time to perform even the first check and thus does not initiate any polling.

The minDelay default is established based on the characteristics of the service and operation, aiming to manage polling frequency efficiently. For DynamoDB, the model sets a minDelay of 20 seconds. This interval is carefully chosen by the service team to avoid excessive polling, which could lead to throttling, or that there is sufficient time for the table creation process to complete before the polling starts.

To align with this, you should ensure that the maxWaitDur is always greater than the minDelay. This setup guarantees that the waiter can perform its intended function and that there is sufficient time to poll the service at least once.

So yes, while minDelay is indeed an optional parameter, maxWaitDur is a required parameter, and users need to be aware of the minDelay default value, and adjust their waiter options accordingly (either by increasing the required maxWaitDur value, or decreasing the optional minDelay value)

While I totally understand why you might want to poll immediately in the first run, the current waiter behavior is in line with our internal spec for waiters, and should be the same across all newer SDKs.
Our developer guide breaks down all the waiter options and actually uses CreateTable and TableExistsWaiter as an example, so I think linking it here is very appropriate.

Thanks again for taking the time and reaching out. I hope this makes sense and doesn't discourage you from submitting additional bug reports/ feature requests in the future.

All the best,
Ran~

@RanVaknin RanVaknin closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

@mmunoz3
Copy link
Author

mmunoz3 commented Mar 19, 2024

Thanks for the comprehensive response. I find the default behaviour cumbersome and somewhat hidden to the user but I understand the reasoning.

Best regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants