add PipeClear

[mkvoya]

Clear the content of a pipeline, including pending calls and/or pending replies. The return value indicates the number of calls/replies cleared and the sign tells calls and replies apart.

[mediocregopher]

Hi there! First off thanks for the submission! I hope the rest of what I'm about to write doesn't discourage you from contributing in the future :P

In this case I wish you had submitted an issue for this first, so you didn't put in work that might not make it in. For small bug fixes I think just going straight to PR is fine, but I'm always very hesitant to add new features to the API, and would have wanted to discuss it first. But no worries, we can discuss it now :)

1) What do you see the use-case for this feature being? Pipe lining, as I see it, is more meant to be an all-at-once kind of thing, i.e. Put a bunch of commands on the pipe at once go, then take off all of their responses at once right after. Which would make this feature not terribly useful. I'm curious what other use cases there might be.

2) Assuming we agree to add this to the API, I think it would make more sense to have the function return two different integers, one for number of outgoing commands dropped and one for incoming responses dropped. The assumption you've made is that one of those numbers has to be zero, but that's not really the case. Someone could do something like this:

client.PipeAppend("INCR", "foo")
client.PipeAppend("INCR", "foo")
client.PipeResp()
client.PipeAppend("INCR", "foo")

At this point there'd be one command on the outgoing queue and one on the incoming queue. This would be a very strange and not-recommended way to use pipelining, but the API allows it so we have to handle it.

Again, thanks for the PR, I hope we can work it out :)

[mkvoya]

Sorry for not submitting an issue at first.

Yesterday when I was using radix.v2 I encountered a situation where I need to check elements in a map and do the "SET" conditionally. So I want to write the following code:

for k,v := range m {
    if v.okToSet {
        client.PipeAppend("SET", k, v.value)
    } else {
        client.PipeClear() // pipe clear is used here to cancel all previous pending SETs
        break
    }
}

Although there is another work-around:

okToSet := true
for k,v := range m {
    if !v.okToSet {
        okToSet = false
        break
    }
}
if okToSet {
    for k,v = range m {
        client.PipeAppend("SET", k, v.value)
    }
}

But maybe I was too lazy to write the for-loop twice. That's why I need a PipeClear. :)

I missed the usage you mentioned above. But it can be handled by changing the return value of PipeClear to a pair of integers, one for the in-coming queue, and the other for the out-coming one.

func (c *Client) PipeClear() (int,int)

I think a PipeClear will make the client more flexible to use. But whether to add such an interface depends on you, since this feature is not that critical (there are many work-arounds).

It's okay for me since I just want to help in such a good library. :)

[mediocregopher]

Ok, that case makes some sense, and @betamike pointed out another case to me that this would also be useful for. If I PipeAppend a bunch of commands, then get the first response. If I only care about the responses for the subsequent commands depending on what the first response is, then clearing out the queue would be required if I decide I don't need them, so this would be helpful there too.

So I guess if it has two separate use-cases then that's a decent argument for adding it :P If you wanna go ahead and do the (int, int) thing, we can start working towards getting this in :)

[mkvoya]

Glad to hear that. :) By the way, I didn't update the doc.go file since I don't know how it is generated. Is it generated by some tools or by hand?

[mediocregopher]

It was written by hand, but don't worry about updating it, it contains information for very general usage, it's not meant to document every single use-case and method.

One final thing, and I'm sorry to be picky about this, but if you could change the doc comment to be:

// PipeClient clears the contents of the current pipeline queue, both commands
// queued by PipeAppend which have yet to be sent and responses which have yet
// to be retrieved through PipeResp. The first returned int will be the number
// of pending commands dropped, the second will be the number of pending
// responses dropped

then I'll merge. Sorry to throw out what you wrote, just like to keep a consistent style to the docs.

[mkvoya]

That's fine. I am not good at English and thanks for your re-writes.

[mediocregopher]

Merged! thanks! :D