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 max open files limit of 512 to rocksdb init #85

Merged
merged 1 commit into from
Jul 19, 2019
Merged

Conversation

kdeme
Copy link
Contributor

@kdeme kdeme commented Jul 16, 2019

Requires status-im/nim-rocksdb#20

Change rocksdb default of -1 to 512, as we reach the 1024 usual (?) open files limit in Linux when running Nimbus at around block ~1.300.000.
I could not immediately find what the default limit is in Windows, so perhaps I just have to test it.

Setting of 512 is somewhat based on value used in parity client: https://github.com/paritytech/parity-common/blob/master/kvdb-rocksdb/src/lib.rs#L197
Should be quite safe, and if needed can probably be increased safely to somewhere between 700-800. Will need incoming connection limitations for that, but that is another issue.

Could also forward this setting to Nimbus + Nimbus config but decided to not yet do this until it is proven to have an practical impact.
I'll create an issue to benchmark + (if needed) tune rocksdb at later moment.

@arnetheduck
Copy link
Member

one thing to consider is tcp connection spamming, and how that affects open files - good to have some margin in general and not push it too far if there's no clear advantage.. ie between 500 and 800 open files on the rocksdb side, what changes? does it have a bigger impact than having 200 instead of 500 spare files for other purposes?

@kdeme
Copy link
Contributor Author

kdeme commented Jul 17, 2019

Yes, that's what I mean with "Will need incoming connection limitations for that, but that is another issue." in my original comment.

I was going to make an issue regarding that. To investigate having some better mechanisms regarding connection limiting.
However, without this change, things are much worse actually, we have no margin at all.

edit: so when I meant safe, I meant safe in the context of this issue and normal "files/sockets open" usage. Not in the context of attacks by keeping pending/lazy connections open and such. That will require more.

@kdeme kdeme merged commit 0d282dd into master Jul 19, 2019
@delete-merged-branch delete-merged-branch bot deleted the max-open-files branch July 19, 2019 11:38
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.

None yet

3 participants