Add ability to do sns-based updates instead of webhooks #1920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ssalinas
commented
Apr 18, 2019
| @@ -572,7 +573,10 @@ public SingularityCreateResult saveTaskHistoryUpdate(SingularityTaskHistoryUpdat | |||
|
|
|||
| @Timed | |||
| public SingularityCreateResult saveTaskHistoryUpdate(SingularityTaskHistoryUpdate taskHistoryUpdate, boolean overwriteExisting) { | |||
| singularityEventListener.taskHistoryUpdateEvent(taskHistoryUpdate); | |||
| Optional<SingularityTask> task = getTask(taskHistoryUpdate.getTaskId()); | |||
There was a problem hiding this comment.
This is the only line I was hesitant about in the PR, but I still think it's a new improvement. With the previous version, we would be doing a write to zk here. Now we are doing a (mostly likely cached) read and a write to SNS/zk. The SNS update falls back to a zk write in case sns is down, so nothing should block here.
ssalinas
commented
Apr 18, 2019
| default: | ||
| break; | ||
| for (WebhookType type : WebhookType.values()) { | ||
| for (SingularityWebhook webhook : webhookManager.getActiveWebhooksByType(type)) { |
There was a problem hiding this comment.
this is a switch to use the cached version of this data since it can be queried frequently
|
🚢 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first in a series of "let's be nicer to zookeeper" PRs. This adds the ability for Singularity to produce messages to SNS instead of backing webhook queues with zookeeper. This should reduce calls to zk by (task updates/second * num subscribers)
TODOs: