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

more efficient serialization of initial state for new worker #1039

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

Keksoj
Copy link
Member

@Keksoj Keksoj commented Nov 24, 2023

As pointed out in #1038, creating a new worker involves writing the config state on a file using serde_json::to_writer:

serde_json::to_writer(&mut state_file, state)

and conversely, using serde_json::from_reader to read the file again, from the new worker process:

let config_state: ConfigState = serde_json::from_reader(configuration_state_file)

which leads to performance issues, especially when the state is big.

This PR fixes this issue by using the same logic as in command sozu state save: convert the ConfigState to WorkerRequests and write them in a file, using a big buffer. Then parse it using nom to retrieve the requests.

This means changing the signature of functions that create workers. Instead of getting a ConfigState, they get Vec<WorkerRequest>.

Gain in performance

  • intel i7-10510U
  • 20000 clusters in the state
  • 100 000 requests in the state file
  • 4 workers
  • upgrade command that restarts all 4 workers with the state file

Sōzu 0.15.17

Executed in   15.74 secs      fish           external
   usr time    7.04 millis    0.00 micros    7.04 millis
   sys time    4.32 millis  820.00 micros    3.50 millis

Sōzu 0.15.17 with the fix

Executed in    7.66 secs      fish           external
   usr time   29.88 millis   29.88 millis    0.00 millis
   sys time   14.20 millis    3.34 millis   10.85 millis

We do observe a doubling of performance, which is nice, but still improvable if we would serialize/parse the requests using protobuf (roughly and additionnal x2 increase in perf).

@Keksoj Keksoj added this to the v0.16.0 milestone Nov 24, 2023
@Keksoj Keksoj self-assigned this Nov 24, 2023
@Keksoj
Copy link
Member Author

Keksoj commented Nov 24, 2023

actually, optimizing worker creation with protobuf does not mean we have to merge this protobuf PR that changes channel behaviour and passes ServerConfig to the worker :

Pull request #1000 changes the type of Config that is passed to the worker, and passes it with a protobuf channel.

The fix of this PR here deals with a temporary file that is read only by the new worker process. We could write protobuf-serialized data in there.

The two pull requests are unrelated.

@Keksoj
Copy link
Member Author

Keksoj commented Nov 27, 2023

This fix here could be improved further by replacing JSON serialization with protobuf, but since it's a protobuf change, it could be done in the big protobuf PR later on.

new function read_requests_from_file
Now, creating a new worker means writing ConfigState as requests
in a file, and parsing them when starting the new worker.
Previously, writing/reading the ConfigStated into a file
was done with serde_json reader and writer, which was inefficient.
Copy link
Member

@Wonshtrum Wonshtrum left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@FlorentinDUBOIS
Copy link
Collaborator

Tested with a state that contains 350k requests and 15k certificates.
Before the patch, it tooks roughly 40s to start and after less than 2s

@FlorentinDUBOIS FlorentinDUBOIS merged commit 8d4e023 into main Nov 29, 2023
13 checks passed
@FlorentinDUBOIS FlorentinDUBOIS deleted the fix-issue-1038 branch November 29, 2023 11:57
Keksoj pushed a commit that referenced this pull request Dec 4, 2023
fix perf issue when creating new worker
@Keksoj Keksoj changed the title fix perf issue when creating new worker more efficitien serialization of initial state for new worker Dec 5, 2023
@Keksoj Keksoj changed the title more efficitien serialization of initial state for new worker more efficitient serialization of initial state for new worker Dec 5, 2023
@Keksoj Keksoj changed the title more efficitient serialization of initial state for new worker more efficient serialization of initial state for new worker Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants