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

Split up webhooks reponse from backend work #1769

Open
tkyi opened this issue Sep 27, 2019 · 6 comments
Open

Split up webhooks reponse from backend work #1769

tkyi opened this issue Sep 27, 2019 · 6 comments
Labels

Comments

@tkyi
Copy link
Member

tkyi commented Sep 27, 2019

What happened: When users have PRs with a lot of file changes or a large number of open PRs, this can cause timeout issues when Github sends webhooks to Screwdriver as it waits for Screwdriver to respond. Github has a 10 second timeout set on their webhooks.

What you expected to happen: Users with large PRs should be able to see a new Screwdriver build without any webhook failures.

Decoupling the listening and reception of the payload, from its processing, is generally the right approach, as I recommended ion "Perl Script slow over Tomcat 6.0 and generates service time out".
The first part should be as fast as possible.

References:

How to reproduce it: Open a PR with >2500 file changes. Check the Github webhooks.

Screen Shot 2019-09-27 at 4 58 25 PM

@catto
Copy link
Member

catto commented Oct 3, 2019

related: #1468

@jithine
Copy link
Member

jithine commented Sep 9, 2021

We should leverage our queue service for this.
Screwdrive application can act as a consumer of the queue as well as producer. Let webhook produce a queue event and publish it to queue service. It should contain enough webhhook data which Screwdriver needs to process it. Then a consumer running in Screwdriver application cluster can process the webhook. This way webhook end point will be super fast, all it needs to do is to produce queue event.

@kumada626
Copy link
Contributor

kumada626 commented Oct 5, 2021

Update 12/1/2021

  • Modify queue-service endpoint instead of create new endpoint.

Proposal

We modify the process of webhook as below.
Current:
スクリーンショット 2021-10-05 10 04 11
Future:
スクリーンショット 2021-10-05 10 04 25

Modified

QueueService /queue/message?type=webhook

This end point receives webhook config from API and queues webhook config to redis queue.

API /v4/webhooks

Currently, this endpoints return response after the build is created.
So modify this endpoints produce a queue event and publish it to QueueSerivce.
In the queue event, it should contain parsed info.
getChangedFiles method uses hook payload in addition to parsed object.
Therefore, queue event also should have this additional payload info.

If user's cluster not using QueueService, it should keep old behavior.

QueueService worker

QueueService worker should polling webhook config.
After polling webhook config, worker should send request to API for processing webhook.

scm-XXX

parseHook method should return additional info for getChangedFiles.
And getChangedFiles should use parsed config instead of raw payload.

New Endpoints

If you have any other good endpoint names, please comment on them.

API /v4/processHooks

In this endpoint, API process webhook.
This is almost the same process that is currently being done in /v4/webhooks after parse hook.

QueueService /queue/webhooks

This end point receives webhook config from API and queues webhook config to redis queue.

Tasks

  • QueueService
    • modify /queue/message?type=webhook endpoint
    • add worker poll webhook config setting
  • scm-XXX
    • parseHook returns additional info for guessing changedFiles
    • getChangedFiles uses parsed config instead of raw payload
  • API
    • create /v4/processHooks endpoint
    • modify /v4/webhooks endpoint

@jithine
Copy link
Member

jithine commented Oct 19, 2021

Why do we need API /processHooks ? Can't QS Worker do the processing itself without needing the API to call /message end point in QS ? It looks like there is lots of back and forth between QS & API.

@kumada626
Copy link
Contributor

When processing webhook, it needs to access to DB. Currently, QS has no config to connect DB and I think it is good to summarize process which needs DB access in API.

@yoshwata
Copy link
Contributor

yoshwata commented Dec 16, 2021

Does anyone know an easy way to get a token for sending a webhook to queueService? The situation is that the build does not exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants