-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
1.0.13 Release Candidate #3076
Comments
Nice release :) |
FWIW, I bumped the couchbase SDK to 1.0.13-SNAPSHOT and all the unit and integration tests look good. Looking forward to the 1.0.13 release, will be shipping with 2.2.0 :) |
This SNAPSHOT badly failed our production canary testing. It seems be causing our application threads to block and grow overtime. @abersnaze is currently doing the tedious bisect + canary test process to figure out which commit is causing the issue. Until we can determine what it is we will not be releasing this version. |
Sorry to hear this. Can you give more details about the failure?
|
Really looking forward to this release 👏 BTW, why isn't the version v1.1.0? This version seems to include significant enhancements. |
So far all new functionality has been added with The change of We definitely are nearing a 1.1.0 release as we have had several Does anyone think anything in this release should be removed from a 1.0.13 release and held off until we're ready for 1.1.0? |
@benjchristensen Thanks to detailed explanations. |
turned out to be |
Thank you @abersnaze for figuring this out. #2969 will need to be reverted, which is proposed in #3081. At this point we don't yet know what the issue is. Do we proceed with 1.0.13 without |
I'd hold up the release because if there is indeed a problem with cache(), fixing it surely involves less drastic changes than a full revert. |
Do you have time to help figure it out in the coming days? Also, we don't need to wait a month until 1.0.14. The delay since 1.0.12 is primarily my fault because I've been busy elsewhere. We can definitely move faster. Another concern I have is your comment about |
I can spare some time to fix cache() but without the usage pattern, I can only guess. Certainly, releasing as is increases a likelihood the problem manifests somewhere else where the devs can create a report with a unit test reproducing the problem. Both backpressure-aware |
We are trying to build a unit test. I believe the issue we're seeing comes from use of We use Hystrix at very high volume, and
Should we revert both |
I've looked at cache() again and it was prone to child-thrown exceptions, breaking the caching process completely. I've fixed it and replay() in #3088 along with other small changes. |
I ran two canaries. The first was with the #3088 and the second with |
Agreed. Let's revert these and move forward with 1.0.13 and release 1.0.14 when these are fixed. |
I've updated the #3081 to rollback both cache and replay. |
@benjchristensen Could you apply, release and undo the "undo" in #3081 so I don't need to start from scratch again? One final test I'd conduct is to revert |
One other thing I can think of is the effect of the |
I think the handling of this issue, distinguishing between experimental vs stable APIs, stability vs features philosophy, usage of Atlas and canary builds would make a nice addition to the Netflix tech blog. So many good practices demonstrated in one place, it's exemplary. |
@akarnokd the test of commit |
I'd check all places which call |
|
Please consider including fix #3091 as well into 1.0.13. |
Merged. The current 1.x branch is what I intend on releasing shortly. |
Update: After production canary testing on the Netflix API we have gained confidence with
merge
and are now ready to release 1.0.13. We did however choose to rollback changes toreplay()
andcache()
and will pursue them in the next release. The discussion in the comments below shares the details.This is a pretty significant release as it has major changes to
merge
and, along with the newreplay
rx.Single
type.The
merge
changes are the most sensitive, as they affect almost all applications (either throughmerge
directly, or the ubiquitous use offlatMap
).I'm going to wait a few days before releasing to give people a chance to use the SNAPSHOT. Use the snapshot from a Gradle build like this:
This release has quite a few bug fixes and some new functionality. Items of note are detailed here with the list of changes at the bottom.
merge
The
merge
operator went through a major rewrite to fix some edge case bugs in the previous version. This has been sitting for months going through review and performance testing due to the importance and ubiquity of its usage. It is believed this rewrite is now production ready and achieves the goal of being more correct (no known edge cases at this time) while retaining comparable performance and memory usage.Special thanks to @akarnokd for this as
merge
is a challenging one to implement.window fix and behavior change
Unsubscription bugs were fixed in
window
. Along the way it also resulted in a fix to one of thewindow
overloads that had a functional discrepancy.This is a small behavior change that corrects it. If you use this overload, please review the change to ensure your application is not affected by an assumption of the previously buggy behavior: #3039
Note that this behavior change only affects that particular overload while the broader bug fixes affect all
window
overloads.rx.Single
After much discussion it was decided to add a new type to represent an
Observable
that emits a single item. Much bike-shedding led to the nameSingle
. This was chosen becauseFuture
,Promise
andTask
are overused and already have nuanced connotations that differ fromrx.Single
, and we didn't want long, obnoxious names withObservable
as a prefix or suffix. Read the issue thread if you want to dig into the long debates.If you want to understand the reasoning behind adding this type, you can read about it in this comment.
In short, request/response semantics are so common that it was decided worth creating a type that composes well with an
Observable
but only exposes request/response. The difference in behavior and comparability was also deemed worth having an alternative toFuture
. In particular, aSingle
is lazy whereasFuture
is eager. Additionally, merging ofSingle
s becomes anObservable
, whereas combiningFuture
s always emits anotherFuture
.Note that the API is added in an
@Experimental
state. We are fairly confident this will stick around, but are holding final judgement until it is used more broadly. We will promote to a stable API in v1.1 or v1.2.Examples below demonstrate use of
Single
.ConnectableObservable.autoConnect
A new feature was added to
ConnectableObservable
similar in behavior torefCount()
, except that it doesn't disconnect when subscribers are lost. This is useful in triggering an "auto connect" once a certain number of subscribers have subscribed.The JavaDocs and unit tests are good places to understand the feature.
Deprecated onBackpressureBlock
The
onBackpressureBlock
operator has been deprecated. It will not ever be removed during the 1.x lifecycle, but it is recommended to not use it. It has proven to be a common source of deadlocks and is difficult to debug. It is instead recommended to use non-blocking approaches to backpressure, rather than callstack blocking. Approaches to backpressure and flow control are discussed on the wiki.Changes
I will be adding examples and more comprehensive release notes for
rx.Single
.These were reverted and won't be included in 1.0.13:
The text was updated successfully, but these errors were encountered: