Skip to content

Conversation

@yunkunrao
Copy link
Contributor

Signed-off-by: rao yunkun yunkunrao@gmail.com

What this PR does / why we need it:

This PR is related with #2467

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? If it is not backward compatible, please discuss on the mailing list first

Signed-off-by: rao yunkun <yunkunrao@gmail.com>
end

if env.ulimit <= 1024 then
print(str_format("Warning! Current user limits [%d] too small, please modify user limits by execute \'ulimt -n <new user limits>\' , otherwise the performance is low.", env.ulimit))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print(str_format("Warning! Current user limits [%d] too small, please modify user limits by execute \'ulimt -n <new user limits>\' , otherwise the performance is low.", env.ulimit))
print(str_format("Warning! Current user limits [%d] is too small, please modify user limits by execute \'ulimt -n <new user limits>\' , otherwise the performance is low.", env.ulimit))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

@tokers tokers changed the title Add ulimit too small prompt feat:add ulimit too small prompt Sep 16, 2021
@yunkunrao yunkunrao requested a review from tokers September 17, 2021 01:35

return function (apisix_home, pkg_cpath_org, pkg_path_org)
-- ulimit setting should be checked when APISIX starts
ulimit = tonumber(util.trim(util.execute_cmd("ulimit -n")))
Copy link
Member

Choose a reason for hiding this comment

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

Should check the nil, err returned from execute_cmd, and log the err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it

end

if env.ulimit <= 1024 then
print(str_format("Warning! Current user limits [%d] is too small, please modify user limits by execute \'ulimt -n <new user limits>\' , otherwise the performance is low.", env.ulimit))
Copy link
Member

Choose a reason for hiding this comment

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

Better to mention what kind of the limit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it

@spacewander spacewander changed the title feat:add ulimit too small prompt feat: add ulimit too small prompt Sep 17, 2021
Signed-off-by: rao yunkun <yunkunrao@gmail.com>
Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

Signed-off-by: rao yunkun <yunkunrao@gmail.com>
Signed-off-by: rao yunkun <yunkunrao@gmail.com>
Copy link
Member

@nic-chen nic-chen left a comment

Choose a reason for hiding this comment

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

should be better add a test case

@spacewander spacewander merged commit 996ad9c into apache:master Sep 21, 2021
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

Successfully merging this pull request may close these issues.

4 participants