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

Add random number generation to Runtime #943

Closed
schreter opened this issue Nov 21, 2023 · 5 comments
Closed

Add random number generation to Runtime #943

schreter opened this issue Nov 21, 2023 · 5 comments
Assignees

Comments

@schreter
Copy link
Collaborator

Currently, openraft is using rand crate to generate random numbers and more concretely, the thread_rng().

Unfortunately, this allocates at suboptimal times, which prevents us from hardening openraft in our project.

The thread-local random number generator query (and its type) should be added to the runtime interface and implemented by default using rand crate.

Alternatively, the random number generator should be created in the local state and not in TLS, which would work as well.

Background: TLS with destructors allocate an entry for running the destructor at thread exit in a vector. This allocation cannot be properly masked in our accounting and we get random extra allocation and/or deallocation accounted making memory leak tests fail.

Copy link

👋 Thanks for opening this issue!

Get help or engage by:

  • /help : to print help messages.
  • /assignme : to assign this issue to you.

@drmingdrmer
Copy link
Member

thread_rng() is only used once in the codebase, when Engine is created. It is called only upon startup. Does it affect the performance?

https://github.com/datafuselabs/openraft/blob/1e062d6d0b4b61f284d42936a56858bfccd839c0/openraft/src/engine/engine_config.rs#L45

@schreter schreter self-assigned this Nov 22, 2023
@schreter
Copy link
Collaborator Author

schreter commented Nov 22, 2023

Does it affect the performance?

No, it's not about performance. It's about the ability to run malfunction and correctness tests of the code.

You don't have to do anything here, I'll take care of it. Just a heads-up.

Maybe I'll do a work-around in our code by instantiating the thread generator manually in the test framework.

BTW, shouldn't it use a newly-generated random number for each election?

@drmingdrmer
Copy link
Member

OK I see.

BTW, shouldn't it use a newly-generated random number for each election?

I'm not sure if it is necessary. At my POV, a random timeout for each node instead of for each round is good enough.
Do not know how to prove that 🤔

@schreter
Copy link
Collaborator Author

Completed by #995

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

No branches or pull requests

2 participants