Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to reset internal ended state #2

Closed
DamonOehlman opened this issue Apr 29, 2015 · 5 comments
Closed

Ability to reset internal ended state #2

DamonOehlman opened this issue Apr 29, 2015 · 5 comments

Comments

@DamonOehlman
Copy link
Member

Just wondering if this would be something that is useful or just plain dangerous. I've tracked down a bug in rtc-signaller (which is really obvious now that I'm looking at it) - see rtc-io/rtc-signaller#35 and it could definitely be solved by allowing me to either:

a. reset the internal ended state of the stream
b. create a new pull-pushable instance transferring the buffer from an old instance

Amongst other things... thoughts? Happy to make a PR if you think either makes sense.

@DamonOehlman
Copy link
Member Author

Actually to clarify, I have been able to fix the issue with no changes to pull-pushable, but believe there is probably a case where buffered messages will go missing without implementing something like this. Not sure if it is worth making a change to pull-pushable though as in reality it does exactly what it should do...

@dominictarr
Copy link
Member

I don't quite understand what you are getting at - do you mean 1) to mark the stream as ended, but if they havn't read everything you are still able to append more items?

Or do you mean 2) to reuse a stream instance that has ended?

  1. sounds maybe reasonable but a bit weird. 2) is out of the question though.

Oh, I think the problem here is that websockets don't have half duplex endings... there is "close" but that is the same as hanging up the phone (either party may hang up, and then neither party can speak)... humans streaming voice into analogue telephony systems found this behavior to be arkward, so they developed a disconnection handshake (I say "goodbye" and wait to get "goodbye" from you, and then either of us can hang up)

I implemented that as a module, specifically for use with websockets pull-goodbye

@DamonOehlman
Copy link
Member Author

Apologies, I don't think I explained this very well at all. I'm not suggesting 1, and I completely agree the 2 is just plain silly (though it is what I suggested in the original issue via suggestion a).

I don't think pull-goodbye is the solution in this particular case (though I could well be wrong), as I'm catering for a websocket being dropped by the server so don't have a polite hangup option.

The scenario is this:

SRC1 -> THR1 -> SNK1

Where SRC1 is a pull-pushable instance, THR1 is a pull.Through stream which is monitoring a stream end condition and reconnecting when appropriate, SNK1 is a pull-ws Sink. Now in the case where SRC1 has already buffered messages [A, B, C] and successfully sent [A, B] before being rudely disconnected by the server. At this point I rebuild the pull-stream flow with:

SRC2 -> THR2 -> SNK2

SNK2 is starting the websocket connection process, and SRC2 is already to go buffering messages and once SNK2 is ok to go those newly buffered messages will start to flow though. The problem is what happens to the buffered message C which was originally queued in SRC1 (now a distant memory). Well at this stage, that message is lost, and while it practically never happens I reckon it probably will at some stage.

I think what I want to be able to do is be able to initialise a new pull-pushable instance, such that any queued messages in SRC1 could be transferred to SRC2, something like:

// create src1
src1 = pushable();
src1.push('A');
src2.push('B');
src3.push('C');

// consume two items from src1 and have it end
...

// initialise a new source cloning any remaining items in src1
// taking into acount the current first arg of creating a pushable is the `onClose` handler
src2 = pushable(null, src1.clone());

That's what I was thinking anyway, which I think makes sense, though I am prone to nonsensical thought...

@dominictarr
Copy link
Member

right! okay I get it you want to have transparent reconnections - this means a client needs to reconnect and then request the log from the last point it got something - it's not good enough for the server to remember what it sent last, because the client may have crashed. or packets may have been dropped.

basically, everything needs a sequence or log number... Quite often there are sequences numbers in the application layer that makes this easy. you could certainly have a queue too, that remembered a buffer of items and the client would send acks to tell the server which things it could forget...

@DamonOehlman
Copy link
Member Author

Yeah thankfully @briely has implemented all this stuff on a newer more robust signaller implementation :)

Closing this for now, because like you say this is probably an application layer concern.

Thanks Dominic :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants