Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bugfix: set random seed for each worker process at init_worker phase, only init phase is not enough. #2357

Merged
merged 3 commits into from
Oct 9, 2020

Conversation

membphis
Copy link
Member

@membphis membphis commented Oct 5, 2020

What this PR does / why we need it:

related PR: #2306

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@membphis
Copy link
Member Author

membphis commented Oct 5, 2020

@zlm0125 welcome to review and make a test with this PR. I think it should work fine for your case

@moonming
Copy link
Member

moonming commented Oct 5, 2020

why only init phase is not enough?

@membphis
Copy link
Member Author

membphis commented Oct 5, 2020

why only init phase is not enough?

init works in master process if the lua_code_cache is on.

We want to set different seed for different work processes, so we should use init_worker phase.

we can delete those code https://github.com/apache/apisix/pull/2357/files#diff-d982d52466e7c93c7b604358339b2a29R85-R90 .
And run the test case, then we will get the same random number which is wrong.

image

Here is the right one:

image

image

@moonming
Copy link
Member

moonming commented Oct 5, 2020

related PR: #2306

I don't understand what it has to do with this PR.
#2306 did not show any details.

@moonming
Copy link
Member

moonming commented Oct 5, 2020

And we should remove random in init phase if add random in init_worker phase.

@membphis
Copy link
Member Author

membphis commented Oct 5, 2020

And we should remove random in init phase if add random in init_worker phase.

you are right. let me do this later.

@membphis membphis merged commit 251625d into apache:master Oct 9, 2020
@membphis membphis mentioned this pull request Oct 9, 2020
4 tasks
@moonming moonming added this to the 2.0 milestone Oct 19, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants