Skip to content

Conversation

@dubee
Copy link
Member

@dubee dubee commented Jan 2, 2020

When an existing consumer is deleted and then quickly recreated, that consumer will get stuck in a dead state. This can happen when using wskdeploy with a manifest that undeploys triggers and then immediately recreates those triggers quickly.

runActionWithExpectedResult(actionName, "dat/multipleValueTypes.json", expectedOutput, false)
}

it should "create a trigger, delete that trigger, and quickly create it again with successful trigger fires" in withAssetCleaner(wskprops) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Test fails when used against provider without the included updates.

@dubee dubee added the review label Jan 2, 2020
logging.info('[{}] Found a change to an existing trigger'.format(change["id"]))

if existingConsumer.desiredState() == Consumer.State.Disabled and self.__isTriggerDocActive(document):
if existingConsumer.desiredState() == Consumer.State.Dead and self.__isTriggerDocActive(document):
Copy link
Member Author

@dubee dubee Jan 2, 2020

Choose a reason for hiding this comment

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

For background info, there is a nanny running periodically that deletes consumers that are in a dead state. Since the consumer is already in a dead state waiting for the nanny to close the process, a quick recreate of the consumer -- within a 2 second time period -- will be ignored. Instead of ignoring the recreate we will manually delete the consumer so that it can be recreated. Normally a graceful shutdown using the nanny is preferred, but we have no other choice here.

# Record the sequence in case the changes feed needs to be
# restarted. This way the new feed can pick up right where
# the old one left off.
self.lastSequence = change['seq']
Copy link
Member Author

@dubee dubee Jan 2, 2020

Choose a reason for hiding this comment

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

Moved this in case of an unexpected exceptions that could happen, allowing us to pick up where we left off.

Copy link

@jasonpet jasonpet left a comment

Choose a reason for hiding this comment

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

LGTM

@jasonpet jasonpet merged commit 566990c into apache:master Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants