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

feat: epoll based worker #139

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Conversation

steve-chavez
Copy link
Member

@steve-chavez steve-chavez commented Aug 15, 2024

This replaces the usage of curl_multi_perform (which uses select()) for curl_multi_socket_action + epoll(), which is more performant (requests finish in less time) and more reliable (with 0 timeouts).

It closes #86. Using the cloud test added on #150, on this branch we get the following results:

postgres=# call repro_timeouts(10000);
NOTICE:  Waiting until 10000 requests complete
NOTICE:  Stats: {"request_successes":10000,"request_failures":0,"last_failure_error":null}
NOTICE:  Time taken: 00:01:01.360567
CALL

postgres=# call repro_timeouts(20000);
NOTICE:  Waiting until 20000 requests complete
NOTICE:  Stats: {"request_successes":20000,"request_failures":0,"last_failure_error":null}
NOTICE:  Time taken: 00:01:44.942479

postgres=# call repro_timeouts(100000);
NOTICE:  Waiting until 100000 requests complete
NOTICE:  Stats: {"request_successes":100000,"request_failures":0,"last_failure_error":null}
NOTICE:  Time taken: 00:08:58.325675

postgres=# call repro_timeouts (1000000);
NOTICE:  Waiting until 1000000 requests complete
NOTICE:  Stats: {"request_successes":1000000,"request_failures":0,"last_failure_error":null}
NOTICE:  Time taken: 01:45:33.085331
CALL

On master:

postgres=# call repro_timeouts(10000);
NOTICE:  Waiting until 10000 requests complete
NOTICE:  Stats: {"request_successes":8886,"request_failures":1114,"last_failure_error":"Ti
meout was reached"}
NOTICE:  Time taken: 00:01:44.061517
CALL
postgres=# call repro_timeouts(20000);
NOTICE:  Waiting until 20000 requests complete
NOTICE:  Stats: {"request_successes":18071,"request_failures":1929,"last_failure_error":"T
imeout was reached"}
NOTICE:  Time taken: 00:04:05.716657
CALL

postgres=# call repro_timeouts(30000);
NOTICE:  Waiting until 30000 requests complete
NOTICE:  Stats: {"request_successes":26939,"request_failures":3061,"last_failure_error":"T
imeout was reached"}
NOTICE:  Time taken: 00:06:31.642828
CALL

So we get 0 timeouts on 1M requests, while on master we got many failures starting at 10K requests. The completion of the requests take much less time as well.

@steve-chavez

This comment was marked as outdated.

@steve-chavez

This comment was marked as outdated.

@steve-chavez
Copy link
Member Author

steve-chavez commented Sep 24, 2024

FYI, there's a sporadic bug on libcurl that leads to some reqs failed: curl/curl#14976

Will put this back to draft until it's resolved.

Reported a bug on curl here: curl/curl#15079

@steve-chavez steve-chavez marked this pull request as draft September 24, 2024 00:41
@steve-chavez steve-chavez marked this pull request as ready for review October 1, 2024 22:09
@steve-chavez
Copy link
Member Author

steve-chavez commented Oct 1, 2024

curl/curl#15079 was solved. It was mostly a lack of documentation. In essence, curl_multi_cleanup shouldn't be done frequently or the curl connection cache will be invalidated and this can cause timeouts (or file descriptor failures in some curl versions). So this should be good to go now.

Fixes sporadic timeouts
@steve-chavez steve-chavez requested review from soedirgo and removed request for soedirgo October 1, 2024 22:17
Copy link
Member

@soedirgo soedirgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

epoll is Linux-only so can't test on my Mac; rubber stamping this

@steve-chavez
Copy link
Member Author

epoll is Linux-only so can't test on my Mac; rubber stamping this

Will open an issue to address this.

@TheOtherBrian1
Copy link
Contributor

Should this be counted as a version change?

@riderx
Copy link

riderx commented Nov 3, 2024

Can't wait this get release in Supabase cloud !

@steve-chavez
Copy link
Member Author

epoll is Linux-only so can't test on my Mac; rubber stamping this

Done on #165

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

Successfully merging this pull request may close these issues.

Webhooks fail when multiple inserts are done
4 participants