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

Revamp to do expiration check entirely in Dynamo to avoid extra GETs/… #23

Merged
merged 12 commits into from
Nov 16, 2021

Conversation

soldiermoth
Copy link

@soldiermoth soldiermoth commented Sep 29, 2021

…DELETEs; add ability to add a cutoff time; redo tests to match

Previously the Lock's workflow was:

  1. GET
  2. If expired DELETE
  3. PUT conditionally
  4. If error sleep RetryWait duration
  5. Repeat 1-4 until no error is returned
  6. Unlock by DELETE

Updated to

  1. PUT conditionally
  2. If error sleep RetryWait duration
  3. Repeat 1-2 until no error is returned or reached cutoff time
  4. Unlock by DELETE

@stuarthicks stuarthicks self-assigned this Nov 12, 2021
@zencoder zencoder deleted a comment from coveralls Nov 12, 2021
@stuarthicks
Copy link

OK I've stared at this long enough that I'm confident I understand this, and I'm happy with the new behaviour (and it works great in tests elsewhere). However, since I've pushed a bunch of commits to this PR, I'm going to tag it for review from another set of eyes before merging.

"github.com/zencoder/ddbsync"
)

func main() {
Copy link

Choose a reason for hiding this comment

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

is this used when we want to manually unlock or something?

Choose a reason for hiding this comment

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

Could be, but I believe the original purpose is to facilitiate manual testing in addition to the unit tests. At least, that's what I've used it for...

@mjh1 mjh1 merged commit b6d126d into master Nov 16, 2021
@mjh1 mjh1 deleted the revamp branch November 16, 2021 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants