-
Notifications
You must be signed in to change notification settings - Fork 188
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
Add ability to do sns-based updates instead of webhooks #1920
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
7dbd7ea
Add ability to do sns-based updates instead of webhooks
ssalinas 6d5df20
make sure webhook fail doesn't block task update
ssalinas 504f798
fallback to zk for retries
ssalinas c780675
travis tweak
ssalinas 673da90
try to fix guice bindings
ssalinas 0cd0c0e
take 2 at guice fixes
ssalinas 6f6318f
fix startup again
ssalinas 92bf9d9
more logging
ssalinas 9ab6a67
translate to webhook object
ssalinas 072daf5
untangle this guice mess
ssalinas 649abda
more guice fun
ssalinas 8a096f3
namespace task webhook retries by request id
ssalinas 89144e6
configurable sns timeouts
ssalinas 90eef2e
Add check for too many webhook child nodes in zk
ssalinas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
121 changes: 121 additions & 0 deletions
121
...larityService/src/main/java/com/hubspot/singularity/config/WebhookQueueConfiguration.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
package com.hubspot.singularity.config; | ||
|
||
import java.util.Map; | ||
|
||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import com.google.common.base.Optional; | ||
import com.google.common.collect.ImmutableMap; | ||
import com.hubspot.singularity.WebhookType; | ||
import com.hubspot.singularity.hooks.WebhookQueueType; | ||
|
||
public class WebhookQueueConfiguration { | ||
@JsonProperty | ||
private WebhookQueueType queueType = WebhookQueueType.ZOOKEEPER; | ||
|
||
@JsonProperty | ||
private Map<WebhookType, String> snsTopics = ImmutableMap.of( | ||
WebhookType.TASK, "singularity-task-updates", | ||
WebhookType.DEPLOY, "singularity-deploy-updates", | ||
WebhookType.REQUEST, "singularity-request-updates" | ||
); | ||
|
||
@JsonProperty | ||
private Optional<String> awsAccessKey = Optional.absent(); | ||
|
||
@JsonProperty | ||
private Optional<String> awsSecretKey = Optional.absent(); | ||
|
||
@JsonProperty | ||
private Optional<String> awsRegion = Optional.absent(); | ||
|
||
private int snsRequestTimeout = 3000; | ||
|
||
private int snsSocketTimeout = 3000; | ||
|
||
private int snsConnectTimeout = 2000; | ||
|
||
private int snsTotalTimeout = 5000; | ||
|
||
// Protection for zookeeper so large list children calls will not take it down | ||
private int maxZkQueuedWebhooksPerParentNode = 3000; | ||
|
||
public WebhookQueueType getQueueType() { | ||
return queueType; | ||
} | ||
|
||
public void setQueueType(WebhookQueueType queueType) { | ||
this.queueType = queueType; | ||
} | ||
|
||
public Map<WebhookType, String> getSnsTopics() { | ||
return snsTopics; | ||
} | ||
|
||
public void setSnsTopics(Map<WebhookType, String> snsTopics) { | ||
this.snsTopics = snsTopics; | ||
} | ||
|
||
public Optional<String> getAwsAccessKey() { | ||
return awsAccessKey; | ||
} | ||
|
||
public void setAwsAccessKey(Optional<String> awsAccessKey) { | ||
this.awsAccessKey = awsAccessKey; | ||
} | ||
|
||
public Optional<String> getAwsSecretKey() { | ||
return awsSecretKey; | ||
} | ||
|
||
public void setAwsSecretKey(Optional<String> awsSecretKey) { | ||
this.awsSecretKey = awsSecretKey; | ||
} | ||
|
||
public Optional<String> getAwsRegion() { | ||
return awsRegion; | ||
} | ||
|
||
public void setAwsRegion(Optional<String> awsRegion) { | ||
this.awsRegion = awsRegion; | ||
} | ||
|
||
public int getSnsRequestTimeout() { | ||
return snsRequestTimeout; | ||
} | ||
|
||
public void setSnsRequestTimeout(int snsRequestTimeout) { | ||
this.snsRequestTimeout = snsRequestTimeout; | ||
} | ||
|
||
public int getSnsSocketTimeout() { | ||
return snsSocketTimeout; | ||
} | ||
|
||
public void setSnsSocketTimeout(int snsSocketTimeout) { | ||
this.snsSocketTimeout = snsSocketTimeout; | ||
} | ||
|
||
public int getSnsConnectTimeout() { | ||
return snsConnectTimeout; | ||
} | ||
|
||
public void setSnsConnectTimeout(int snsConnectTimeout) { | ||
this.snsConnectTimeout = snsConnectTimeout; | ||
} | ||
|
||
public int getSnsTotalTimeout() { | ||
return snsTotalTimeout; | ||
} | ||
|
||
public void setSnsTotalTimeout(int snsTotalTimeout) { | ||
this.snsTotalTimeout = snsTotalTimeout; | ||
} | ||
|
||
public int getMaxZkQueuedWebhooksPerParentNode() { | ||
return maxZkQueuedWebhooksPerParentNode; | ||
} | ||
|
||
public void setMaxZkQueuedWebhooksPerParentNode(int maxZkQueuedWebhooksPerParentNode) { | ||
this.maxZkQueuedWebhooksPerParentNode = maxZkQueuedWebhooksPerParentNode; | ||
} | ||
} |
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.