Skip to content

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

Merged
membphis merged 3 commits into
apache:masterfrom
membphis:bug-random
Oct 9, 2020
Merged

bugfix: set random seed for each worker process at init_worker phase, only init phase is not enough.#2357
membphis merged 3 commits into
apache:masterfrom
membphis:bug-random

Conversation

@membphis

@membphis membphis commented Oct 5, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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

membphis commented Oct 5, 2020

Copy link
Copy Markdown
Member Author

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

@moonming

moonming commented Oct 5, 2020

Copy link
Copy Markdown
Member

why only init phase is not enough?

@membphis

membphis commented Oct 5, 2020

Copy link
Copy Markdown
Member Author

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

moonming commented Oct 5, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

related PR: #2306

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

@moonming

moonming commented Oct 5, 2020

Copy link
Copy Markdown
Member

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

@membphis

membphis commented Oct 5, 2020

Copy link
Copy Markdown
Member Author

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

you are right. let me do this later.

moonming
moonming reviewed Oct 5, 2020
View reviewed changes
Comment thread apisix/init.lua
Comment thread apisix/init.lua
@membphis
membphis merged commit 251625d into apache:master Oct 9, 2020
@membphis membphis mentioned this pull request Oct 9, 2020
Closed
4 tasks
@moonming moonming added this to the 2.0 milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spacewander spacewander spacewander approved these changes
@moonming moonming moonming approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@membphis @moonming @spacewander