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

[PR] Freeze all operators with same priority and warn #114

Closed
2 tasks done
kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Closed
2 tasks done

[PR] Freeze all operators with same priority and warn #114

kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Labels
archive enhancement New feature or request

Comments

@kopf-archiver
Copy link

kopf-archiver bot commented Aug 18, 2020

A pull request by smileisak at 2019-06-14 19:27:12+00:00
Original URL: zalando-incubator/kopf#114
Merged by nolar at 2019-07-08 12:42:49+00:00

One-line summary

Issue : #51

Description

When two operators runs with the same priority, both will handle the same request. This PR adds the possibility to freeze all events with the same priority, as mentioned in #51 and print a warning error.

Types of Changes

What types of changes does your code introduce? Keep the ones that apply:

  • Bug fix (non-breaking change which fixes an issue)

Review

List of tasks the reviewer must do to review the PR

  • Manual Tests
  • Documentation

Commented by smileisak at 2019-06-26 13:20:56+00:00
 

samurang87 let me know if i need to change something in this PR.

Thank you


Commented by nolar at 2019-07-03 17:49:15+00:00
 

smileisak Can you please resolve conflicts with the new master branch? (Or maybe give me the write access to your fork.) — And then I will merge it.

I guess, it is only the move kopf.reactor.peearing -> kopf.engines.peering here (see #124).


Commented by smileisak at 2019-07-05 10:14:03+00:00
 

nolar this breaks the CICD Pipeline, i sent you an invitation to have write access to my fork.


Commented by nolar at 2019-07-05 10:58:39+00:00
 

smileisak Oh! That's an interesting issue. It seems, it existed there since the beginning, and was only revealed now.

When an operator exits, it does not remove its record from the peering object. When the next operator starts fast enough (which is the case for the e2e tests), it believes that another operator is running, and freezes.

Previously, it was not a problem, since all test-operators were same-ranked, and the freeze was ignored (though the potential conflict was logged).

This is caused by how asyncio task cancellation works: it is a CancelledError exception injected into a task (expected), but it also implicitly cancels all other coroutines/tasks created after the injection (unexpected) — and so, the peering self-deletion was also silently cancelled.

Now, with this new commit, the self-deletion is "shielded" from the cancellation of its calling task. And also logs if another exception has happened.


Commented by nolar at 2019-07-08 13:09:46+00:00
 

Released in kopf==0.18

smileisak Thanks once again for your contribution.

@kopf-archiver kopf-archiver bot closed this as completed Aug 18, 2020
@kopf-archiver kopf-archiver bot changed the title [archival placeholder] [PR] Freeze all operators with same priority and warn Aug 19, 2020
@kopf-archiver kopf-archiver bot added the enhancement New feature or request label Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

0 participants