Check enforcer event consolidation #1005
Closed
+159
−75
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
mitchdenny
added
EngSys
damirault
reviewed
Sep 17, 2020
|
|
||
| var everythingElse = from payload in elligiblePayloads | ||
| group payload by payload.CheckRun.HeadSha into payloadGroup | ||
| select payloadGroup.Last(); |
There was a problem hiding this comment.
Order by "completed_at" to get the most recent one? Might be useful for future caching optimizations.
damirault
reviewed
Sep 17, 2020
|
|
||
| if (delta > 0) | ||
| { | ||
| Debugger.Break(); |
There was a problem hiding this comment.
Focusing on this delta I think is the most interesting thing. Are we hitting this often enough or do we really need to focus on caching the identical API calls themselves (and off box?).
A drop in distributed cache might be easy enough.
damirault
reviewed
Sep 17, 2020
| var context = new HandlerContext<T>(deserializedPayload, client); | ||
|
|
||
| await HandleCoreAsync(context, cancellationToken); | ||
| // TODO: Handle exceptions on deserialization (too big, malformed, shouldn't |
There was a problem hiding this comment.
The various failure and retry cases seems like the biggest open question with this implementation. Goes directly to the retry logic in the event-hub and a per/multi message basis.
kurtzeborn
added
the
Central-EngSys
|
Don't need to do this. Dropping neutral events had a massive impact. I think we've got plenty of headroom now. |
sima-zhu
pushed a commit
to sima-zhu/azure-sdk-tools
that referenced
this pull request
Dec 3, 2020
* Update the storage readme with concepts * Update app -> application Move the context into the options to be consistent with C++ and the context being optional for callers of convenience layer APIs * Information on how to replace the http stack
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 PR is an exploration of one approach to reducing the volume of requests being routed to GitHub from Check Enforcer. It works by deduping messages that are received from EventHubs as a batch. After some experimentation I am not sure that this approach will work that well (the delta reduction is not that great).
We might need to do a more stateful approach.