Hi All, Firstly, I really appreciate the care and attention everyone is putting into this. I'm finding this challenge fun to navigate.
Secondly, I don't feel so silly anymore because clearly this is a non-trivial task. I'm going to have to take a bit to let the last couple of suggestions sink in and figure out how to apply them in this case. Too tired to completely grok them tonight (I feel you Egon!) I'm starting to think there is some value in just sleeping temporarily (in the test only of course), even though I find this to be a less than desirable practice. In this case it may be the least complex and most reliable way. That said, Egon and Augusto are absolutely correct. That test avoids validating that no new requests enter the RLock following close, even if it successfully validates that all requests which had already entered RLock are completed. Putting the done channel check inside RLock makes the current code work, but the value of the test is both ensuring the current code works as desired, as well as future refactors. It leaves us wide open for someone to make the mistake I made when transcribing to Go Playground. So the simple "sleep" solution validates one case, but leaves me with another potentially more difficult case to validate. To touch on Augusto's point, I agree that it's less than desirable to have requests that "accidentally" made it in after close is called wait for all other tasks to complete before getting a proper HTTP response. The production implementation of "Close" actually waits for the listener to close and for http.Serve to exit completely before attempting to take a lock on the tasks; however, since Serve is launching goroutines, Serve may exit with a connection in the state between the call to the handler and taking out the RLock. In our production environment the risk of that should be minimal because procedure dictates that the traffic is shut off before gracefully shutting down the process. That's all fine and good as a procedure, but leaves the code in a state where it doesn't meet the requirements without proper ops procedures. This may be a tradeoff that we accept, but leave things precarious for future developers and ops who inherit this code. Okay, time for me to sleep! Thanks again everyone for the time you put into this. Evan On Tuesday, 13 September 2016 22:41:44 UTC-7, Egon wrote: > > On Wednesday, 14 September 2016 00:58:39 UTC+3, Evan Digby wrote: >> >> Hi Egon, >> >> Thanks for that. It seems to implement the same requirements as >> implemented in my example, although I prefer my implementation as it >> doesn't require a sleep/loop. >> > > I used sleep/loop because I probably was thinking about sleep too much at > that time :D. It could have been done with a WaitGroup. > > Anyways, coming back to the problem at hand... > As I said there are two properties that a good concurrency primitive > should have: > > 1. It should be easy to understand. > 2. It should have a formal or an informal proof. > > Tests are less useful, unless they are supported by the scheduler. If they > are not, then you pretty much either end up with quite tests that don't > cover the "hard stuff" -- e.g. the problem in your initial code. > > Now you can write the tests that implement the program state-machine... > crude version https://play.golang.org/p/wBTbA2bQTP ... then write your > whole program in terms of Routine -- where each func is an "atomic" > instruction... then write the Step function that it goes through all > possible orderings... or alternatively use randomized testing and run > through only N different orderings... Put as you can imagine it's pretty > annoying. *(Just a top of the head idea.)* > > Now there are "testing frameworks" that can help here as well... e.g. > https://www.youtube.com/watch?v=zi0rHwfiX1Q ... There's > https://golang.org/pkg/testing/quick/ that provides some support for > it... i.e. you need to add an "Action" generator and a "State" primitive... > and each call to Check does a random action and verifies whether "State" is > in a consistent state. *(You may need to add an additional func > "WouldWait" to do it.)* > > For actually verifying/proving -- people involved with Raft have came up > with easy-to-use framework, relatively speaking... > http://verdi.uwplse.org/ ... to say the least, proper verification > requires significant effort. > > Here's the simplest I was able to make the needed primitive > https://play.golang.org/p/MkmarqPzr1 ... and instead of testing "Do" and > "Finish" you test the rest of the interface -- they are deterministic -- > and non-racy. (Using testing/quick here would be probably ideal.) > > + Egon > > >> The implementation I provided in the original email, with the exception >> of the bug that Augusto pointed out, works as desired. The question was >> not how to implement these requirements, but how to validate them with a >> test cleanly. >> >> Unfortunately it doesn't seem that I was clear in my original post since >> most people are focusing on the implementation of the requirements. >> >> Thanks again for your effort in this--apologies for miscommunication the >> original question. >> >> Evan >> >> >> On Tue, 13 Sep 2016 at 14:48 Egon <egon...@gmail.com> wrote: >> >>> >>> >>> On Wednesday, 14 September 2016 00:18:26 UTC+3, Evan Digby wrote: >>>> >>>> Hi Egon, >>>> >>>> My requirements are more simple than a graceful http shutdown. I simply >>>> require that everything that enters the RLock completes to RUnlock. >>>> Accepted requests, or even calls to servehttp can die without issue as >>>> long >>>> as they haven't entered the processing in the RLock. >>>> >>> >>> In that case https://play.golang.org/p/RiFEbQvytP. *Ps. Still tired, >>> may contain bugs.* >>> >>> >>>> If the server crashes we have ways to deal with that, but it's a more >>>> DevOps-y process. Recovering from logging, etc.,and should be an edge case >>>> that I'm not worried about handling in code. >>>> >>>> If the handler gets stuck in a loop, we will see that in our logging. I >>>> don't want the server to die in that case. I want it to keep retrying (we >>>> have exponential backoff) and informing us via structured logging of >>>> what's >>>> going on. If it's an unanticipated loop/block, then there will be a manual >>>> investigation into the server's state before we manually kill the process. >>>> At that point it becomes similar to the last point, except easier because >>>> we already know the state the message was in. >>>> >>>> In our use case we will always exit shortly after a close. It's safe to >>>> assume the process will die after close returns. >>>> >>>> Thanks again, >>>> >>>> Evan >>>> >>>> On Tue, 13 Sep 2016 at 14:08 Egon <egon...@gmail.com> wrote: >>>> >>> On Tuesday, 13 September 2016 23:31:55 UTC+3, Evan Digby wrote: >>>>>> >>>>>> Hi John/Egon/Augusto, >>>>>> >>>>>> I should point out is that all we need to guarantee (barring abnormal >>>>>> termination of course) is that once a task starts processing, it >>>>>> finishes. >>>>>> Partially processed messages are bad, but http requests that don't >>>>>> result >>>>>> in a message being processed at all are okay. >>>>>> >>>>>> We don't need to guarantee that the result of every Accept in the >>>>>> HTTP server results in a processed message. We handle this on the OPS >>>>>> side >>>>>> by ensuring we stop sending requests to that instance before terminating >>>>>> the process. We just want to make sure, at that point, that the messages >>>>>> which did make it to the handler are flushed. >>>>>> >>>>>> So the case where: >>>>>> >>>>>> h.Handle(...) <-- gets past the closed channel check, calls go ..., >>>>>> butthe >>>>>> goroutine doesn't execute yet. >>>>>> h.Close() <-- closes the close channel, Locks and Unlocks,returns. >>>>>> ...now the goroutine executes and acquires the read lock. >>>>>> >>>>>> We actually don't care if "Handle" completes in this example. We only >>>>>> care if that our task handler starts processing a message that it >>>>>> completes >>>>>> the processing. >>>>>> >>>>> >>>>> How do you actually ensure that it completes processing without >>>>> hooking into Server? I.e. that buffers and sockets get properly flushed? >>>>> >>>>> Let's take a step back and what are the properties that you need -- >>>>> >>>>> I assume it's just graceful shutdown where all the pending ServeHTTP >>>>> requests have been processed? >>>>> >>>>> What should happen when the server crashes -- is it vital for those >>>>> requests to be processed, once they have been accepted? >>>>> >>>>> What should happen when one handler gets stuck in an infinite >>>>> wait/loop? >>>>> >>>>> Does the "Close" returning mean you exit main or does hpw does the >>>>> process termination depend on it? Or is it just another goroutine that is >>>>> terminating not the whole process? >>>>> >>>>> + Egon >>>>> >>>>> -- >>>>> You received this message because you are subscribed to a topic in the >>>>> Google Groups "golang-nuts" group. >>>>> To unsubscribe from this topic, visit >>>>> https://groups.google.com/d/topic/golang-nuts/jh-nvt9ukBg/unsubscribe. >>>>> >>>> To unsubscribe from this group and all its topics, send an email to >>>>> golang-nuts...@googlegroups.com. >>>> >>>> >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> -- >>> You received this message because you are subscribed to a topic in the >>> Google Groups "golang-nuts" group. >>> To unsubscribe from this topic, visit >>> https://groups.google.com/d/topic/golang-nuts/jh-nvt9ukBg/unsubscribe. >>> To unsubscribe from this group and all its topics, send an email to >>> golang-nuts...@googlegroups.com. >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
