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

Send fake heartbeats to verify channel is still open for long running workers #28

Merged
merged 6 commits into from
Feb 20, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 5, 2019

No description provided.

@ghost ghost self-assigned this Feb 5, 2019
@ghost ghost requested review from ezzatron and jmalloc February 5, 2019 00:08
Copy link
Contributor

@jmalloc jmalloc left a comment

Choose a reason for hiding this comment

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

A few questions, but looks good generally :)

@ezzatron we would like to test this with you to verify it actually solves the post-docker-sleep-slow-service-recovery problem.

src/Amqp/DeclarationManagerInterface.php Outdated Show resolved Hide resolved
@@ -142,7 +142,12 @@ public function run()
foreach ($this->procedures as $procedureName => $procedure) {
$this->unbind($procedureName);
}
}
} //else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this else should be uncommented?

@ghost ghost requested a review from jmalloc February 5, 2019 01:20

interface DeclarationManagerInterface
{
public function exchange();
Copy link
Contributor

Choose a reason for hiding this comment

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

Any need to retain exchange() on this interface?

@ghost ghost requested a review from jmalloc February 20, 2019 22:56
@jmalloc jmalloc merged commit 53fedc4 into develop Feb 20, 2019
@jmalloc jmalloc deleted the send-fake-heartbeats branch February 20, 2019 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant