Sample operator: last item before termination is dropped after sampling

[kboyarshinov]

Hi. Recently I found that sample operator drops last value before onCompleted event. This is behaviour is valid according to these discussions #566 and #571 and this test. It originated in Rx.NET and still exists in RxJava to be consistent across all implementations as far as I understood.

But, marble diagram in documentation shows otherwise: item 5 is emitted before termination and sampled by sampler.

This behaviour is odd and unexpectable. I stumbled across it trying to resolve backpressure issue with observables where last emitted item is the most important.

So, my question is whether it can be changed? I prepared a commit with fixes and corresponding tests.
Or at least marble diagram should be changed to showcase this behaviour and it would be good to have a note about it in documentation.

Thank you.

[akarnokd]

What about when the source completes and the sampler doesn't?

[kboyarshinov]

@akarnokd it would emit the last item from completed source and nothing after that, no?

[akarnokd]

Right, the diff tricked me. I expected 3 calls to emit, 1 in sampler's onNext, 1 in sampler's onCompleted and 1 in parent's onCompleted.

Besides, I can see that operator still has some issues with unsubscription and backpressure management.

[kboyarshinov]

Yep, 2 calls is enough: 1 in sampler's onNext and 1 in parent's onCompleted.

[akarnokd]

So if the sampler completes, then the value should be dropped just like now?

[kboyarshinov]

It is ok in my usecase. If sampler completes before source completes it means no values are expected before completion. But if source completes before sampler does it should emit last value.

However, it worth discussing.

[akarnokd]

See #3658 that contains other fixes.

[kboyarshinov]

Great! Also OperatorSampleWithTime will require fixes too.

[akarnokd]

Would you like to submit a PR?

[kboyarshinov]

@akarnokd I would like to. Will submit soon.

[kboyarshinov]

Done #3757

[akarnokd]

#3757 merged.