Skip to content

Conversation

@luke-jr
Copy link
Member

@luke-jr luke-jr commented Oct 4, 2011

-blknotifypidfile option to send SIGUSR1 to a PID when there's a new best block

Used by basically every pool now, it just sends SIGUSR1 to a poolserver.

@gavinandresen
Copy link
Contributor

Does this work on Windows?

@TheBlueMatt
Copy link
Contributor

Better question - does this even compile on Windows?

@luke-jr
Copy link
Member Author

luke-jr commented Oct 5, 2011

Good question. No idea. I don't use Windows... Anyone want to step up to test, or should I resubmit this with non-Windows #ifdefs?

@shadders
Copy link

shadders commented Oct 6, 2011

I didn't think there was such a thing as SIGUSR1 on windows.

Whilst this is the current implementation many pushpool pools use now it has a couple of limitations that are relevant to pools and some extras that are specific to java based pools (i.e. poolserverj).

1/ it requires the pool engine to be on the same machine as the bitcoin daemon
2/ Java is flaky enough with SIGUSR1 and cannot determine which pid the signal came from so for a java recipient of the signal it requires that they either listen to only 1 bitcoin daemon or that it not care which daemon the signal comes from. This is not the case for poolserverj and wouldn't be for any pool engine that uses multiple bitcoin daemons.
3/ If indeed it isn't windows incompatible that's another negative from poolserverj's side since it can run as a windows service. There is at least small pool using it on windows.

The principal is good and definitely desirable from a pool engine's perspective, it's just the method of sending the signal that is limiting.

poolserverj currently uses an intermediary mini-daemon (written in C) that listens for signals then sends a signal over a network socket. This is a very simple but custom protocol. Open socket, send a \n terminated string [passphrase]:[bitcoind id string] then close socket. This is pretty hackish so I'd be open to an alternative but I think a network based notification method would be preferable for the above reasons.

perhaps in keeping with the json-rpc trend
Perhaps -blknotifycallbackurl= with a json-rpc method: notifyblockchange

@laanwj
Copy link
Member

laanwj commented Oct 11, 2011

I do think bitcoin needs a mechanism to send asynchronous notifications of incoming transactions, new blocks, whatever a client wants to subscribe to.

However, I agree with @shadders, and don't think signals are the preferred interprocess communication method here. Signals are specific to UNIX and pretty annoying to handle at that, as they interrupt system calls, and have many other tricky corner cases.

@d33tah
Copy link

d33tah commented Oct 14, 2011

How about dbus?

@luke-jr
Copy link
Member Author

luke-jr commented Oct 14, 2011

Dbus is a desktop technology. This would primarily be used on servers...

@laanwj
Copy link
Member

laanwj commented Oct 14, 2011

DBUS, featurewise would be exactly what we need here. It supports both
synchronous and asynchronous calls and async notifications/subscriptions, so
we could implement the entire bitcoin API plus notifications in it. It also
has some rudimentary security features which could come in useful.

However it also has some drawbacks:

  • It is bound to one machine: I have tried in the past to use it as a kind
    of remote RPC by tunneling over ssh, but is was a lot of hassle and you end
    up exposing all the machine's services not just the one you want
  • It is mainly a Linux desktop technology. It needs the dbus daemon to be
    running on the local machine to work at all. I haven't seen many active DBUS
    installs on Windows, Mac, or (as LukeJr says) on servers.
  • I'm not sure about web language support. Is there DBUS for PHP? Well maybe
    there is, but it is not very common.

There is a large compatiblility argument for "Keeping it HTTP" here. On the
other hand, our current JSON-RPC doesn't support asynchronous notifications
or subscribing to them, so we'd have to bolt it in ourself with callbacks,
which could also be a bad idea: more to maintain, more "bitcoin-specific"
shit, I'd ideally like to use proven 3rd party libraries when possible.

Any ideas? :-)

On Fri, Oct 14, 2011 at 7:12 AM, Luke-Jr <
[email protected]>wrote:

Dbus is a desktop technology. This would primarily be used on servers...

Reply to this email directly or view it on GitHub:
#555 (comment)

@TheBlueMatt
Copy link
Contributor

I completely agree with luke here. DBus really isnt the best choice here, primarily because it is really mostly used as a desktop technology. Additionally, does java have a good DBus library? One of the complaints about sending a USR1 signal was poor java support therefor. Although this is largely platform-specific (again, does this compile on windows?) I would say this, it is still worth merging.

"Wladimir J. van der Laan" [email protected] wrote:

DBUS, featurewise would be exactly what we need here. It supports both
synchronous and asynchronous calls and async
notifications/subscriptions, so
we could implement the entire bitcoin API plus notifications in it. It
also
has some rudimentary security features which could come in useful.

However it also has some drawbacks:

  • It is bound to one machine: I have tried in the past to use it as a
    kind
    of remote RPC by tunneling over ssh, but is was a lot of hassle and you
    end
    up exposing all the machine's services not just the one you want
  • It is mainly a Linux desktop technology. It needs the dbus daemon to
    be
    running on the local machine to work at all. I haven't seen many active
    DBUS
    installs on Windows, Mac, or (as LukeJr says) on servers.
  • I'm not sure about web language support. Is there DBUS for PHP? Well
    maybe
    there is, but it is not very common.

There is a large compatiblility argument for "Keeping it HTTP" here. On
the
other hand, our current JSON-RPC doesn't support asynchronous
notifications
or subscribing to them, so we'd have to bolt it in ourself with
callbacks,
which could also be a bad idea: more to maintain, more
"bitcoin-specific"
shit, I'd ideally like to use proven 3rd party libraries when possible.

Any ideas? :-)

On Fri, Oct 14, 2011 at 7:12 AM, Luke-Jr <
[email protected]>wrote:

Dbus is a desktop technology. This would primarily be used on
servers...

Reply to this email directly or view it on GitHub:
#555 (comment)

Reply to this email directly or view it on GitHub:
#555 (comment)

@d33tah
Copy link

d33tah commented Oct 14, 2011

How about the callback URL's then?

@TheBlueMatt
Copy link
Contributor

At that point its getting much too complicated for what it is designed to do.
If we use SIGUSR1 some applications (like poolservj) will have to implement a wrapper which receives the signal and forwards it on in another method.
If we use callback URLs then other applications either get much more complex or write a wrapper which receives the callback and forwards on a SIGUSR1.
There really is no perfect way, so IMHO the best solution is the simplest for us that will work for some others, which IMO would be SIGUSR1.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 14, 2011

They don't have to be exclusive, either...

@TheBlueMatt
Copy link
Contributor

True, though I would really prefer to KISS here instead of implementing a large new addition for such a simple feature.

@shadders
Copy link

why does it have to be one or other? I callback side by side with a
sigusr1 offers both and is trivial to implement on the bitcoin side
given that it already implements a json-rpc client. It could be easily
configurable to use one or the other or both. The most common
application that needs it (i.e. poolservers) already implement http
listeners and go as far as json-rpc listeners.

If you want to wrap sigusr you are limited in the languages you can use
to do it and you are forced to maintain an additional daemon (which
particularly in java's case breaks platform independence). If you don't
you are limited to receiving push notifications on the same machine.

On 15/10/11 01:28, Matt Corallo wrote:

At that point its getting much too complicated for what it is designed to do.
If we use SIGUSR1 some applications (like poolservj) will have to implement a wrapper which receives the signal and forwards it on in another method.
If we use callback URLs then other applications either get much more complex or write a wrapper which receives the callback and forwards on a SIGUSR1.
There really is no perfect way, so IMHO the best solution is the simplest for us that will work for some others, which IMO would be SIGUSR1.

@shadders
Copy link

it's not exactly complicated... a http call. And if the framework is in
place can easily be extended to any either type of event notification.
Sigusr doesn't provide any granularity. Unless it goes to a different
pid for every possible permutation of message. As far as I know you
cannot add any sort of arbitrary data to the signal to distinguish it.
I'm far from knowledgable on signals though so I'm happy to stand corrected.

On 15/10/11 01:37, Matt Corallo wrote:

True, though I would really prefer to KISS here instead of implementing a large new addition for such a simple feature.

@freewil
Copy link
Contributor

freewil commented Oct 14, 2011

This may be something more thorough than what is needed, but some sort of hooks "interface" like @laanwj was saying, for events like new transactions, blocks, etc is much needed. I imagine something flexible like being able to specify in the conf file an event and a script to run or a callback URL to POST to.

@shadders
Copy link

I think a script call would be just as usable as a callback url. A
little more fiddly but more flexible if the end user doesn't want to
implement a http listener.

On 15/10/11 01:44, freewil wrote:

This may be something more thorough than what is needed, but some sort of hooks "interface" like @Iaanwj was saying, for events like new transactions, blocks, etc is much needed. I imagine something flexible like being able to specify in the conf file an event and a script to run or a callback URL to POST to.

@freewil
Copy link
Contributor

freewil commented Oct 14, 2011

Yup, what @shadders says. A script to run (probably passed some helpful arguments) or a callback URL I think gives the best flexibility.

@laanwj
Copy link
Member

laanwj commented Oct 14, 2011

Yes a callback script or URL would have my preference for now. This could do
anything, including send a signal. And when the system is in place it could
be re-used for other kinds of events later on. This provides much more
flexibility than a fixed signal to a specific PID.

@mikegogulski
Copy link

Gavin had some ideas about a similar topic way back here: #20

I also agree that doing things non-portably (such as via SIGUSR1, as proposed) isn't the right way to go. If a stopgap is needed prior to a full JSON-RPC callback implementation, I'd suggest something like simply writing the new block's hash to a file that can act as a semaphore. Other programs could just stat() this file periodically for its last update time, as a simple IPC.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 14, 2011

A lot of talk as if this is merely a proposed thing, but it's in fact the de-facto standard for pretty much every longpoll-enabled miner.

@shadders
Copy link

That's a polling not push. The fallback mechanism for 'listeners' that
can't use SIGUSR and don't want to implement a half-node is to poll
getblocknumber already.

On 15/10/11 05:55, Mike Gogulski wrote:

Gavin had some ideas about a similar topic way back here: #20

I also agree that doing things non-portably (such as via SIGUSR1, as proposed) isn't the right way to go. If a stopgap is needed prior to a full JSON-RPC callback implementation, I'd suggest something like simply writing the new block's hash to a file that can act as a semaphore. Other programs could just stat() this file periodically for its last update time, as a simple IPC.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 15, 2011

The ideal solution would probably be native longpolling in bitcoind. The file-based mechanism would be push, if you use inotify to monitor it for changes ;)

@shadders
Copy link

Not the case. Poolserverj is now used by 2 of the 4 largest pools +
numerous small pool. The 2 largest that don't use poolserverj are using
proprietary pool server and I couldn't say whether they use longpoll
signals or not. It was the standard and when there was only 1 option
for OSS pool software it met everyones needs but PSJ's adoption has
pretty clearly demonstrated how limiting the solution is.

In terms of backward compatibility and script to send the same signal is
a one liner.

On 15/10/11 06:37, Luke-Jr wrote:

A lot of talk as if this is merely a proposed thing, but it's in fact the de-facto standard for pretty much every longpoll-enabled miner.

@shadders
Copy link

On 15/10/11 10:00, Luke-Jr wrote:

The ideal solution would probably be native longpolling in bitcoind. The file-based mechanism would be push, if you use inotify to monitor it for changes ;)

//inotify/ is a Linux kernel subsystem that acts to extend filesystems
to notice changes to the filesystem, and report those changes to
applications./

Do you see the problem here?

@mikegogulski
Copy link

Suggestion: resolving/merging this should have the lowest possible priority. Why? It benefits ~2 dozen pool operators. Dev attention is far better (as in by orders or magnitude) spent on improving the end-user experience and extending capabilities for commerce.

Alternately, surround the relevant code with "#ifdef poold" or something, pull it, and let pool operators build their own. Case closed, on to more important things.

@shadders
Copy link

To suggest that needs of pool operators are unimportant because there
only '~2 dozen' of them is incredibly short sited. Those 2 dozen are
hubs for 90% of the hashing power that secures the bitcoin network. Do
you really think there would be as many miners if they all had to mine
solo and wait 2 years between payouts? The existence of pools makes
bitcoin more secure than it would be without them.

Furthermore the change does not benefit just the pool operators it
benefits the miners of which there thousands. Those miners also happen
to be a very large proportion of the end users at the moment. It also
benefits the network as a whole by making pooled miners more efficient
which again means more hashing power.

No one is asking anyone who doesn't care to do anything. This
discussion is simply to resolve the most future proof way of enabling a
push notification mechanism for which this change is only 1 use case.
If you don't have any interest don't take part in the discussion but
what possible benefit is there in suggesting others shouldn't discuss it?

On 15/10/11 17:00, Mike Gogulski wrote:

Suggestion: resolving/merging this should have the lowest possible priority. Why? It benefits ~2 dozen pool operators. Dev attention is far better (as in by orders or magnitude) spent on improving the end-user experience and extending capabilities for commerce.

Alternately, surround the relevant code with "#ifdef poold" or something, pull it, and let pool operators build their own. Case closed, on to more important things.

@shadders
Copy link

p.s. since commerce capabilities meet your classification of 'important'
consider this. Once a push notification mechanism is adopted an obvious
commerce use case is notification of new txs. User makes purchase on
cart, goes to checkout and gets bitcoin address to pay, sends payment.
Cart site receives push notification when payment arrives. User may well
be waiting on final checkout page for confirmation payments is
received. For a large site with many concurrent checkouts in progress,
each session polling perhaps once/sec (since you don't want to give a
bad user experience with long delays) is a horrible solution compared to
a simple listener.

On 15/10/11 17:00, Mike Gogulski wrote:

Suggestion: resolving/merging this should have the lowest possible priority. Why? It benefits ~2 dozen pool operators. Dev attention is far better (as in by orders or magnitude) spent on improving the end-user experience and extending capabilities for commerce.

Alternately, surround the relevant code with "#ifdef poold" or something, pull it, and let pool operators build their own. Case closed, on to more important things.

@d33tah
Copy link

d33tah commented Oct 15, 2011

I agree with shadders - the "lowest possible priority" thing is just an obvious misunderstanding of the concept - the IPC code, if implemented correctly, could give benefits to all bitcoind admins - making Bitcoin e-commerce easier.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 15, 2011

Shadders, I consider it a non-issue that Java is too broken to support standard POSIX functionality. Lack of Windows support for SIGUSR1 specifically is also arguably a non-issue, since nobody sane runs servers on Windows anyway-- I suppose I need to add some #ifdefs though. If people actually need something like this on Windows, I'm sure it has some non-standard equivalent too.

@mikegogulski
Copy link

"IPC code, if implemented correctly, could give benefits to all bitcoind admins"

No, only to those running bitcoind on the very same machine as their commerce app/pool server -- or running such an app at all. RPC solves the general case, and those running both on the same box can point to localhost.

@shadders: Did you actually read issue 20?

@freewil
Copy link
Contributor

freewil commented Oct 15, 2011

Here is gaven's actually work on this, it's probably outdated, but something like this - added with the option of running a script also would solve the problem and make everyone happy I think.

gavinandresen@f62c608

@shadders
Copy link

On 16/10/11 04:09, Mike Gogulski wrote:

@shadders: Did you actually read issue 20?
Of course I read it... Are you referring to this part?:
"/Used by basically every pool now/"

Unfortunately that's only true in luke's happy place where JVMs don't
exist. What he really means is every pool running pushpool or a
derivative which is a minority by a very big margin.

/>>> Shadders, I consider it a non-issue that Java is too broken to support standard POSIX functionality.

/

Luke currently 75% of network hash power is served by pools that use a
JVM. That's hardly an inconsequential minority.

There are two solutions presented here that are both simple to implement
(both use mechanisms already present in bitcoind), extensible,
cross-platform, cross-machine and can easily be made backward
compatible. It would appear the only advantage you could gain by
choosing to go with SIGUSR1 over either of them is to lock out 'broken
java' and non-posix compliant platforms.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 16, 2011

Please explain how SIGUSR1 notifications (the topic here) have anything to do with your far more complicated solutions. It's my understanding that your ideas are not in any way exclusive or broken by getting this simple implementation in.

@freewil
Copy link
Contributor

freewil commented Oct 16, 2011

It's my understanding that your ideas are not in any way exclusive or broken by getting this simple implementation in.

I think the goal should be to not include quick solutions like this for every single need in the official release. IMO a far more flexible and comprehensive (therefore superior) solution has been proposed.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 16, 2011

"Superior" can be based on a lot of different things. Every other proposed solution requires significantly more code and CPU time to implement, whereas this is a simple standard solution to a simple problem.

@freewil
Copy link
Contributor

freewil commented Oct 16, 2011

Every other proposed solution requires significantly more code and CPU time to implement, whereas this is a simple standard solution to a simple problem.

If you were able to specify a script/executable to be called upon a "newblock" event, and that script/executable simply created the same signal, would this really take up so much more cpu time to be detrimental?

@shadders
Copy link

On 16/10/11 11:15, Luke-Jr wrote:

Please explain how SIGUSR1 notifications (the topic here) have anything to do with your far more complicated solutions. It's my understanding that your ideas are not in any way exclusive or broken by getting this simple implementation in.

Because the solutions achieve the same goal with none of limitations,
significant extra benefits and minimal extra code.

Aside from that it's simple politics. Once you implement a 1/2 arsed
solution that satisfies some people it becomes that much harder to get
support for a complete solution. The few that were satisfied by the 1/2
solution no longer care because their needs are met which leaves the
remaining unsupported people to argue for the change on their own.

You're calling these complicated solutions but I honestly can't see
what's hard about it. Both probably require an extra line or two of
code to parse the arguments. And probably one or two more to execute
the rpc call or script. The rest is already done.

Why is executing an external script so complex for this application when
you seem perfectly ok with it in your coinbaser patch? That is a far
more complex use of external scripts since it involves waiting on the
returned output and parsing it. This is simply a 'fire and forget' case.

As I've said if SIGUSR1 is important to you there's no reason you
couldn't send the signal at the same time. But the rest of the work is
done already so it's only these few lines of code to be added and all
potential consumers of the service are satisfied.

Let me break it down for you...

SIGUSR1:

  • 75% of pool hash power is excluded from benefiting. + more for
    those that don't run bitcoind on the same machine as pushpool.
  • limited to one event type per target pid.
  • Those that are able to use it already do.
  • Specific to one and only one use case

Script/RPC

  • Easily backwards compatible (script calls 'kill' or RPC has side by
    side SIGUSR1 option)
  • 100% of pools can benefit
  • extensible to many use cases that are non-pool related.
  • cross-platform, cross-machine
  • perhaps 4 extra lines of code within existing patch.

@luke-jr
Copy link
Member Author

luke-jr commented Oct 16, 2011

Unlikely. I've never used coinbaser's TCP mode outside of testing, simply because popen wasn't a bottleneck ;)

@luke-jr
Copy link
Member Author

luke-jr commented Oct 16, 2011

By complex, I was mainly referring to JSON/HTTP-based solutions, which require an implementation of both of those specs-- neither part of the common C standard.

As implied, I did implement TCP support into coinbaser because of the possibility of overhead from executing an external script. I also found it wasn't an issue, so never ported my Eligius code to make use of it. I can also see and agree that most pools, even pushpool-based, could benefit greatly from a coinbaser-like system/popen solution rather than SIGUSR1-- their script could send the signal and then start work on processing payouts.

If someone wishes to implement the system/popen solution (really will probably be a fork-and-exec so it can be a simple fire-off), I'd be glad to close this in favour of merging that. Not much difference between -blknotifypidfile=foo.pid and -blknotify='kill -USR1 $(<foo.pid)'

@jgarzik
Copy link
Contributor

jgarzik commented Dec 19, 2011

SIGUSR1 is an ugly hack on pushpool's part. However, it is also very simple and straightforward. With more complex solutions stalled, I think pragmatism says it would be OK to merge this with some minor changes:

  1. rename cmdline option to some Unix-y, like "-sigusr1file"

  2. make sure the code builds on Windows (translation: #ifdef out)

@luke-jr
Copy link
Member Author

luke-jr commented Dec 19, 2011

Superceded with pull #714

@luke-jr luke-jr closed this Dec 19, 2011
ptschip pushed a commit to ptschip/bitcoin that referenced this pull request May 11, 2017
Fix potential deadlock - cs_xval
rajarshimaitra pushed a commit to rajarshimaitra/bitcoin that referenced this pull request Aug 5, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants