/FrameworkBenchmarks

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

Update facil.io version #3259

Merged
merged 7 commits into from Feb 7, 2018

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@boazsegev @michaelhixson @nbrady-techempower
@boazsegev
Contributor

boazsegev commented Feb 6, 2018

I doubt if this is necessary, but Michael mentioned there were issue with the facil.io run (or, to be precise, that something happened on the app server during, or just before, the facil.io run).

The updated version mainly focused on the Websocket client features and minor refactoring, but the minor refactoring included changes to the logic in the beta.2 fix for cluster mode, so maybe this will help.

I should probably point out that I stress tested the beta.4 today with no adverse effects (no memory leaks, no crashes, all was good). However, I have no way to test an 80 core cluster with remote clients (I just don't have that kind of access), so I'm not sure if this fixes whatever went wrong (if it's even facil.io related).

Sidenote: I removed the 0.0.0.0 binding from the command line. The NULL address should support IPv6, while I fear the 0.0.0.0 might limit the address scheme to IPv4 (I doubt it, but I never tested the idea).

Bo added some commits Feb 1, 2018

Bo Bo
Link the framework test to the updated facil.io version 
Reviewing the testing logs exposed the fact that facil.io detected 80
CPU cores on the host machine (?!)… which invoked 80 forks.

This large number of forks was never attempted before and it exposed a
number of issues which were fixed in the beta.2 release.

This release also includes other minor updates and fixes which are
probably irrelevant (such as updated to the pub/sub and a fix for the
Redis engine etc’).
34cdfa6
Bo Bo
Merge remote-tracking branch 'TechEmpower/master'
2ded5bc
Bo Bo
Bump facil.io version
ef5af81
Bo Bo
Update facil.io version 
I doubt if this is necessary, but Michael mentioned there were issue
with the facil.io run (or, to be precise, that something happened on
the app server during, or just before, the facil.io run).

The updated version mainly focused on the Websocket client features and
minor refactoring, but the minor refactoring included changes to the
logic in the beta.2 fix for cluster mode, so maybe this will help.

I should probably point out that I stress tested the beta.4 today with
no adverse effects (no memory leaks, no crashes, all was good).
However, I have no way to test an 80 core cluster with remote clients
(I just don't have that kind of access), so I'm not sure if this fixes
whatever went wrong (if it's even facil.io related).

**Sidenote**: I removed the `0.0.0.0` binding from the command line.
The NULL address should support IPv6, while I fear the `0.0.0.0` might
limit the address scheme to IPv4 (I doubt it, but I never tested the
idea).
ece55d9
Bo Bo
Update facil.io version 
I doubt if this is necessary, but Michael mentioned there were issue
with the facil.io run (or, to be precise, that something happened on
the app server during, or just before, the facil.io run).

The updated version mainly focused on the Websocket client features and
minor refactoring, but the minor refactoring included changes to the
logic in the beta.2 fix for cluster mode, so maybe this will help.

I should probably point out that I stress tested the beta.4 today with
no adverse effects (no memory leaks, no crashes, all was good).
However, I have no way to test an 80 core cluster with remote clients
(I just don't have that kind of access), so I'm not sure if this fixes
whatever went wrong (if it's even facil.io related).

**Sidenote**: I removed the `0.0.0.0` binding from the command line.
The NULL address should support IPv6, while I fear the `0.0.0.0` might
limit the address scheme to IPv4 (I doubt it, but I never tested the
idea).
1cf3ab9
Bo Bo
Update to beta 5, for enhanced error handling for large clusters
f8b1237
@michaelhixson

This comment has been minimized.

Show comment
Hide comment
@michaelhixson

michaelhixson Feb 6, 2018

Member

LGTM assuming it works on Travis. I tested this on ServerCentral, and it fixes the issue we're seeing with the current build of facil.io.
Member

michaelhixson commented Feb 6, 2018

LGTM assuming it works on Travis. I tested this on ServerCentral, and it fixes the issue we're seeing with the current build of facil.io.
Bo Bo
Limit the number of cores utilized to 7 worker cores ( + 1 sentinel) 
With credit to Michael for point this out… at some point the added
processes don’t really help as much as they use up resources.
e1c4107

@nbrady-techempower nbrady-techempower merged commit 7f70b5e into TechEmpower:master Feb 7, 2018
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment