search

erlware / relx

Sane, simple release creation for Erlang
http://erlware.github.io/relx
Apache License 2.0
697 stars 232 forks source link

use random:uniform instead of os:pid when constructing node name in nodetool #868

Closed hmmr closed 3 years ago

hmmr commented 3 years ago

Borrowing from https://github.com/basho/node_package/blob/4.0/priv/base/nodetool#L195, this is to help reduce the risk of hitting the atom table limit, as was reported by one of our customers who was calling riak-admin continuously and frequently enough to trigger the atom table overflow.

tsloughter commented 3 years ago

This looks good, thanks. I'll probably merge soon.

But I wanted to note that in OTP 23+ nodetool is no longer used and this issue does not exist. Obviously still worth it to be fixed for those using pre-23, just wanted to mention it :)

hmmr commented 3 years ago

@tsloughter Indeed, I read that note and slightly pondered if it's worth while bothering. But, on reflection, it seems it still does :)

tsloughter commented 3 years ago

Hm, the shelltestrunner tests and the tests on windows fail.

ferd commented 3 years ago

Yes, definitely should replace with the newer stuff where available.

hmmr commented 3 years ago

@tsloughter After the approval, what is the current state of this PR? Is there anything I can do to help?

tsloughter commented 3 years ago

Hey, sorry about that. I don't know what the hell is going on with CI... there is at least 1 other PR that should be passing CI but isn't that I also want to merge and cut a release with.

tsloughter commented 3 years ago

Could you repush so it kicks of CI again? There isn't even a "rerun" option anywhere like there usually is...

hmmr commented 3 years ago

Once it's in, there's a more substantial #871.