Added debugging logic for our stopper pattern.

[tiger5226]

Added debugging logic for our stopper pattern. Useful for when a stopper doesn't stop right away as expected.

[lyoshenka]

I'd prefer not to have code like this in the library. It adds extra complexity and it's not something most people want most of the time. For one thing, go already provides tools for debugging. For another, the code in this is so simple that you could copy/paste it locally and add debugging statements if you wanted to. Finally, I haven to seen any other go library do things like this.

[tiger5226]

I have had to use it a number of times now, and figured instead of rewriting it every time, I would just commit it this time. It is VERY useful when there is a lot of concurrency, like in chainquery.

When using the stopper pattern, you link the stopping of the application to a specific subset of go routines. Knowing which go routine, where, is not ending fast enough, makes it really easy to debug and find the root cause.

go already provides tools for debugging

Here the documentation referenced is clear how to get similar information. Niko and I did this once, but I can tell you this is MUCH easier and quicker than that method. There is no "figuring out" which routines are the applicable ones and no tracing the stack is required because the logging is functional.

adds extra complexity

I think this is a minimal impact and the user of the library can choose to do this based on their needs.

it's not something most people want most of the time

Minimal concurrency makes this added overhead, yes, but it is not required for the stopper to be used. The situations where you do not need this, can just use the standard methods. Although, I would argue it is still useful for when you get into a situation where a stopper doesn't stop right away.

this is so simple that you could copy/paste it locally and add debugging statements if you wanted to

Agreed, but I think if you find yourself re-writing the same thing over and over again, you might as well just finalize it. This being added to the library, makes it a 1 line change for me to turn on debugging of the stopper. Every time I close a routing it reports what else it is waiting on to finish closing.

I haven't seen any other go library do things like this

I would be interested in an equally beneficial way of doing something similar. The key items I am looking for:

• A functional identification of dependant go routines ( ie the console statement that tells me exactly what this go routine is for ).
• A simple way to turn it on without modifying dozens of lines of code.
• When not enabled it has little to no impact on the application.

Different isn't always bad either. Domain specific debugging tactics can be priceless. This is just one specific to the stopper pattern. Not saying it can't be done better, but it works and fulfills it's expectations well.

I would argue that sometimes people think too generically. For example, look here. General debugging tactics. I get it, there are tools available. However, my need is very specific. delve is also another tool. This is overkill for me, but it is a generally acceptable and generic solution for debugging a golang app. I want to debug the stopper, quickly and easily.

If you still think this is too specific to my use case, I can remove this from the stopper library, just wanted to plead my case. When things are perfect, debugging is never needed, when there is a problem, you wish you had a switch like this to turn on.

[lyoshenka]

Can you show me an example of where/how this is used?

[tiger5226]

this is an example. In the most recent example, the stopper was taking up to a minute to stop the application. So I added this and it told me right away the problem was in the mempoolsync and it was the go routine the stopper was waiting for. There are 2 dozen go routines running if I were to dump the app. I only care about the ones required to end with a graceful shutdown. Know, which ones the stopper is waiting on specifically, is really helpful output. In this implementation, I just change a line of code to turn it on.

[lyoshenka]

i thought about it for a while but couldn't come up with a better approach that was also simple. i did have a few implementation suggestions and bugfixes: https://github.com/lbryio/lbry.go/pull/49

• rename the functions to DebugAdd and DebugDone so its clear that they are meant for debugging
• use mu instead of l for the mutex variable. its what ive seen used in the go standard libraries, and its technically more accurate (its a mutex, not a lock)
• i suggest not makeing the map until DebugAdd is called. then it is not made in the typical non-debug case (and you don't have to call a separate constructor.
• i didn't do this, but you should add comments to DebugAdd and DebugDone

[lyoshenka]

and now that ive thought about it some more, i think i misunderstood what you were trying to do with NewDebug. i see how it makes sense to leave AddNamed and DoneNamed in place, and simply turn on the debug behavior by changing the constructor. ignore that part of my suggested changes.

[tiger5226]

Ok I adjusted the other PR based on these comments.