v1: change_stream (CXX-3237, CXX-3238) #1538
Open
+1,203
−218
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves CXX-3237 and CXX-3238 for the
v1::change_streamcomponent.This component was tricky to deal with due to having upwards of three distinct states being tracked by both the change stream object and its iterators, despite the iterator state supposedly intended to be tracked by the associated change stream object (hence the "shared state" of associated iterators):
v_noabi::change_stream::impl::status_: one ofk_pending,k_started, andk_dead.v_noabi::change_stream::impl::exhausted_:bool.v_noabi::change_stream::iterator::_type: one ofk_tracking,k_default_constructed, andk_end.After careful examination and testing, the
v1implementation narrows the state space down to only the following three exclusive states, which are shared by all associated objects (there is no state exclusive to iterators):is_active: the change stream (iterator) has an event document available.is_resumable: the change stream (iterator) has no event document available.is_dead: the change stream (iterator) encounter an irrecoverable error.Compare the differences in v1 and v_noabi's implementation of
advance_iterator()anditerator::operator==()for further details, where the consequences of these changes are most clearly visible.An important difference between v1 and v_noabi behavior to highlight is that consecutive calls to
.begin()do advance the shared iterator state in v1, whereas v_noabi carves a special exception to permit consecutive calls to.begin()to leave the shared iterator state unchanged until the next explicitoperator++. This exception is not inherited by the v1 API due to being inconsistent with similar stateful iterator API (that is, calling.begin()don't usually have an "unless" in their advance-the-iterator behavior). Nevertheless, this behavior is preserved by the v_noabi API for backward compatibility.Additional notes:
v1::change_stream::iteratordocumenting the moved-from state as assign-or-destroy-only, the implementation is trivial (equivalent to copy). This is intentional to leave open the possibility of changing_implto have its own class impl; state rather than completely depending on the associated v1::change_stream object. If this is YAGNI, we can make all the SMFs implicitly trivial instead (Rule of Zero), but left as-is for now.const_castrequired to preserve API backward compatibility with the incorrect declaration ofbegin() const, which does modify thechange_streamobject. The v1 API fixes this declaration, but consequently, v_noabi must accomodate for this fix in its refactored implementation. Relevant incorrect-const workarounds are labeled with descriptive comments._is_endstate to preserve consecutive-.begin()-does-not-advance behavior.scan-buildonly warns about a potential null pointer dereference inv_noabi::change_stream::iterator::is_exhausted()and not anywhere else not-null is assumed.