A quiet ReactorMongoSubscriptionModel subscription kept a position the oplog could drop - #1181
Draft
johanhaleby wants to merge 10 commits into
Draft
johanhaleby wants to merge 10 commits into
johanhaleby wants to merge 10 commits into
Conversation
…e oplog could drop The reactive driver hands over the documents of a change stream and nothing for an empty batch, so a subscription that matched no events kept the position of the last one that did. A pause and a resume or a restart after a long quiet period then opened at a position MongoDB no longer had. The model now sends aggregate and getMore itself, on a session it opens for each change stream, since MongoDB refuses a getMore on another session than the cursor's. A reply with no document moves the position to its post batch resume token, and the reactor QuietPositionReportingSubscriptions hands that position to a listener. A named subscription reads in runs. A new run for an id reads nothing until the previous run has closed and its action or listener Mono has completed or been cancelled, and the next batch is asked for only once the actions for the batch before it have completed.
The changelog, the upgrade guide and ADR 142 now cover the reactor model. They say it reads from the primary whatever the read preference, that waitUntilStarted() waits for MongoDB to open the change stream, and that ReactorDurableSubscriptionModel doesn't save the quiet position yet.
Sending aggregate and getMore from the model failed with CursorNotFound behind two mongos routers. A subscription with an id now reads the driver's change stream cursor one batch at a time, and takes the postBatchResumeToken from the private wrapped field of BatchCursor. When that field can't be read, the model logs a warning and reads through ReactiveMongoTemplate.changeStream(..) as before. waitUntilStarted(), the read preference, the driver's own resume, the plain Flux and the lost-history check are back to what main does.
ADR 142, the changelog and the upgrade guide described the cursor the model read itself, with its own session, always from the primary.
…ubscription-position
A token read that failed came back as no token, so a driver whose getter kept failing left a subscription without a quiet position and logged nothing. Now only a read while the driver opens the change stream again gives no token, which the model tells apart by the cursor the driver reads with. Any other failure makes the model read through Spring's changeStream with the one warning. The look at the token is only safe because every field it reads through is final or volatile in the driver. The model now checks those modifiers when it loads and for every cursor, and falls back the same way when one is missing.
ADR 142 said the model read the token from a plain field and relied on HotSpot for the order of two reads. On driver 5.8.0 every field on the way is final or volatile, so a later look reads a reply at least as new as an earlier one. The ADR, the changelog and the upgrade guide now say so, and say that a missing modifier or a failing token read ends in the warning too.
One test fails when the driver of this build declares a field on the way to the token as neither final nor volatile. Two more break the driver's token read with a ByteBuddy agent. A read that starts failing after the cursor opened gives one warning and events through Spring's change stream. A read while the driver's cursor reference is empty gives no warning and quiet positions keep coming.
A CheckpointWriteConditionNotFulfilledException from a quiet position handler ended delivery for the subscription, which still answered isRunning(id) with true. Nothing in the library raises it from a reactor quiet handler, since the reactor stack has no lease, so only a listener of your own could stop a subscription that way. The model now reads again from the subscription's position after its backoff, as for any other error from that handler, and an action's refused write is retried as on main. The two tests that asserted the old outcome now assert that a handler refusing once lets the subscription deliver the next event, and that one that keeps refusing makes the model read again from the quiet position. The test of the driver's reopen waits for four looks instead of four seconds, so a slow machine still gets three token reads in.
The reactor QuietPositionListener javadoc and ADR 142 said a CheckpointWriteConditionNotFulfilledException from a quiet position handler ended delivery on the node. The model now retries it forever, like an error from an action, by reading again from the subscription's position. Both now say so, and the ADR says why the blocking models still end delivery on it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #1169
A
ReactorMongoSubscriptionModelsubscription that matched no events kept the position of the last event that did match, so a pause and a resume, a restart of the change stream or a lease handover after a long quiet period opened the change stream at a position the oplog had dropped. The reactive driver hands over the documents of a change stream and nothing for an empty batch, so the model never saw the newer position MongoDB sends with each batch.A subscription with an id now reads the driver's own change stream cursor one batch at a time, and moves its position to the
postBatchResumeTokenof a reply that had no event for it. A listener can get that position through the new reactorQuietPositionReportingSubscriptions, the counterpart of the blocking capability from #1174.I opened this as a draft.
ReactorDurableSubscriptionModeldoesn't save the quiet position yet, since #1179 changes that class and is still open. Until it does, a process restart after a long quiet period can still end in lost history for a reactor durable subscription. The list of what it needs is under Still open.How the model gets the token
ChangeStreamPublisherhas no method that returns thepostBatchResumeToken. The model opens the change stream with the publicBatchCursorPublisher.batchCursor(int), which is the method the driver calls itself when something subscribes to the publisher. It then reads the driver's change stream cursor, anAsyncChangeStreamBatchCursor, from the private fieldBatchCursor.wrapped, and callsgetPostBatchResumeToken()on it. It also reads the private fieldAsyncChangeStreamBatchCursor.wrapped, theAtomicReferencethat holds the cursor the driver reads with, to tell when the driver is opening the change stream again. The model never writes either field. Every batch and the close go through the publicBatchCursor.next()andBatchCursor.close(), so the commands, the server they go to, the session and the driver's resume after a failover are the driver's own, as withReactiveMongoTemplate.changeStream(..).The model checks the route in three places:
DriverChangeStreamCursorlooks upBatchCursorPublisher.batchCursor(int),AsyncAggregateResponseBatchCursor.getPostBatchResumeToken(), and getters forBatchCursor.wrappedandAsyncChangeStreamBatchCursor.wrappedwithMethodHandles.privateLookupIn(..). It also checks thatBatchCursor.wrapped,AsyncChangeStreamBatchCursor.wrappedandCommandCursorResult.postBatchResumeTokenare final. Any failure becomes the reason in one WARN, and the model reads every subscription throughReactiveMongoTemplate.changeStream(..)as on main.AsyncChangeStreamBatchCursor, and the class of the cursor inside it has to declare a volatilecommandCursorResult. If not, the model closes that cursor, logs the WARN once and reads throughReactiveMongoTemplate.changeStream(..)from then on.AsyncChangeStreamBatchCursor.wrappedfor that and then puts a new cursor in, so a look that finds it empty, or that fails and then finds it empty or holding another cursor, gives no token. The check doesn't read the exception's message.Either way the events delivered are the same, and only the quiet position is lost. Two tests in
ReactorMongoSubscriptionModelQuietPositionTest,the_change_stream_cursor_of_the_driver_can_be_read_with_the_driver_of_this_buildanda_subscription_that_has_reported_a_quiet_position_still_reads_through_the_cursor_of_the_driver, fail the build when the route is off on the driver version the build uses.The driver jars have no
module-info, only anAutomatic-Module-Name, and an automatic module opens every package. I checked it with a named module on the module path on Temurin 21.0.12.1, whereprivateLookupIn(..)andsetAccessible(true)both work onBatchCursor.wrapped, andprivateLookupIn(..)works onAsyncChangeStreamBatchCursor.wrapped.Why the model doesn't send
aggregateandgetMoreitself anymoreThe previous version of this PR read its own cursor with commands. On a sharded cluster with two
mongosrouters, itsgetMorecommands went to whichever router the driver picked, and over 20 seconds 8 of 21 failed withCursorNotFoundand it opened the change stream 8 times. The driver's change stream on the same cluster opened it once, and none of its 20getMorecommands failed.ReactorMongoSubscriptionModelTwoMongosTeststarts that cluster in onemongo:8.0container, which took 4 to 5 seconds locally and 4.6 seconds on CI with both JDK 21 and JDK 25. The whole class took 31 and 28 seconds on CI. Against the previous version of this PR,a_subscription_opens_its_change_stream_oncefailed with[change streams opened] Expected size: 1 but was: 12, anda_subscription_sends_no_getMore_that_failsfailed on thegetMorecommands that gotCursorNotFound. The other two tests of the class, that a quiet position is reported and that a matching event is delivered, pass on the previous version too. All four pass on this version.Invariants
Monoor a listener's, has completed or been cancelled.Monohas completed for every event of the batch before, so no batch is fetched ahead. Within one call tonext()the driver sends anothergetMoreonly after a reply with no document, and the reply that ends the call comes last. A token that a later look in the same call finds replaced therefore came with a reply that had no document. The model never moves to the token it finds at the latest look, since the driver stores a reply before it hands over its documents.ReactiveMongoTemplate.changeStream(..), and only the batch size of the first batch and whennext()is called differ.No lock is held across the caller's code. Each wait is a
Mono.Invariant 2 also needs a later look to read a reply at least as new as an earlier look did, although the driver stores it on another thread. On driver 5.8.0 every field on the way is final or volatile.
BatchCursor.wrappedis final,AsyncChangeStreamBatchCursor.wrappedis a finalAtomicReference,AsyncCommandCursor.commandCursorResultis volatile andCommandCursorResult.postBatchResumeTokenis final. On 5.5.2 the cursor inside is anAsyncCommandBatchCursor, whosecommandCursorResultis volatile too. The looks of one wait come one after the other. Those modifiers are private to the driver, which is why the model checks them and falls back when one is missing.Paths
subscribe(..)validates the filter and the start position, registers the subscription, and subscribes the run outside the monitor. The run waits for every earlier run for the id.waitUntilStarted()completes when the change stream is subscribed to, as on main.Monohas completed.killCursors. A move after the close counts only while a step that started before it is still under way.restartSubscriptionsOnChangeStreamHistoryLost(true). Otherwise the run ends and the subscription is forgotten, as on main. The driver's cursor hands theMongoCommandExceptionover unwrapped, so main's one-level check finds it.CheckpointWriteConditionNotFulfilledExceptionincluded, by reading again from the subscription's position after the backoff rather than in place. That loses nothing, since the position only moves to a token a later look found replaced.ReactiveMongoTemplate.changeStream(..).ReactiveMongoTemplate.changeStream(..)from its position. During the reopen the look gives no token.Back to main's behaviour
The previous version changed more than the quiet position needed. These are main's behaviour again:
killCursorswhen the run closes the cursor.waitUntilStarted()completes when the change stream is subscribed to.ReactiveMongoTemplate.changeStream(..)does, instead of always the primary.Fluxofsubscribe(filter, startAt)is main's code, with no quiet position. Nothing listens for it.One change stays. The model asks for the next batch only after the actions of the batch before have completed. Invariant 2 needs it, since a batch fetched ahead moves the token past events the action hasn't had. It costs a round trip per batch, and the changelog and the upgrade guide say so.
What changes for callers
Monohas completed for every event of the batch before it.changeStream(..)on a mockedReactiveMongoOperationsno longer reaches a subscription with an id, since the model opens it fromgetCollection(..). Two tests ofReactorMongoSubscriptionModelResilienceTeston main stubgetCollection("events")too, and their assertions are unchanged.The changelog has these under Changes and Breaking changes, and section 19 of the upgrade guide has a subsection for them. ADR 142 has a section for the reactor model.
ApplyFilterToChangeStreamOptionsBuilder.changeStreamPipeline(..)in the common module now builds the filter stages bothSpringMongoSubscriptionModeland the reactor model put after$changeStream, which the Spring model built itself before. Its output for the Spring model is the same.Tests
mvn -pl <modules> -am testwith the tests of the modules this PR touches, on Temurin 21 against colima. The reactor module ran on 396d02d. The other rows are from b1e7f34, before the token read could fall back and before a refused quiet position write was retried, and I didn't run them again:subscription-mongodb-spring-reactorsubscription-mongodb-spring-reactor-checkpoint-storagesubscription-mongodb-spring-blockingsubscription-mongodb-spring-blocking-checkpoint-storagesubscription-mongodb-spring-blocking-competing-consumer-strategysubscription-durable-reactorsubscription-catchup-reactorsubscription-stream-catchup-reactordcb-dsl-reactorprojection-dsl-reactormongodb-reactive-spring-boot-starterNew test classes in the reactor module:
ReactorMongoSubscriptionModelQuietPositionTest, 16 tests of the quiet position itself, pause and resume, the listener, the runs for one id,killCursorsand the session of everygetMore, the two route tests above, and a test that the driver of this build declares every field on the way to the token final or volatile.ReactorMongoSubscriptionModelTokenWatchTest, 7 tests of the look rule.ReactorMongoSubscriptionModelRunTest, 8 tests of when a run moves the position and runs a step.ReactorMongoSubscriptionModelDriverCursorTest, 10 tests of lost history with and without a restart, aCheckpointWriteConditionNotFulfilledExceptionfrom a quiet position handler once and every time, the fallback toReactiveMongoTemplate.changeStream(..)with its single warning, and two token reads that fail. A ByteBuddy agent in the test sources makesgetPostBatchResumeToken()on the driver's change stream cursor throw. In one test every read throws after the cursor opened, and the model warns once and delivers the next event. In the other the test emptiesAsyncChangeStreamBatchCursor.wrappeduntil the model has looked four times, so at least three token reads find it empty, and the model gives no warning and reports quiet positions after it.ReactorMongoSubscriptionModelDriverResumeTest, 3 tests that the driver opens the change stream again after its connection is closed and the model doesn't restart the subscription.ReactorMongoSubscriptionModelTwoMongosTest, 4 tests on the two-router cluster.The only change to a test from main is the two
getCollection("events")stubs inReactorMongoSubscriptionModelResilienceTest. The previous version of this PR changedReactorMongoSubscriptionLifecycleTestandReactorMongoSubscriptionModelResilienceTestmore, and both are main's again apart from those stubs. I deleted two tests the previous version had added,a_plain_subscription_that_restarts...andwaiting_until_a_subscription_has_started..., since they asserted behaviour this version takes back to main's.A few of the failures I got when I broke the code the new tests check:
wrappedX, the route test failed withexpected: null but was: "java.lang.NoSuchFieldException: no such field: …BatchCursor.wrappedX/…".[getMore sent while the action runs] Expected size: 1 but was: 9.[the third run started while the first run is being cancelled] Expecting value to be false but was true.equals(..)instead of by instance, a TokenWatch test failed withExpecting actual: [null, null] to contain exactly …[null, {"_data"=BsonString{value='a'}}].[the change stream opens at the resume token the driver holds] Expecting value to be true but was false.AsyncCommandCursor.batchSize, it failed withprivate int com.mongodb.internal.operation.AsyncCommandCursor.batchSize isn't volatile. Pointed atAsyncChangeStreamBatchCursor.resumeTokenas a field that has to be final, it failed withprivate volatile org.bson.BsonDocument …AsyncChangeStreamBatchCursor.resumeToken isn't final.[warnings that the resume token can't be read] Expected size: 1 but was: 0.[events delivered] Expecting actual: [] to contain exactly (and in same order): ["64cdf14a-…"], and the test whose handler keeps refusing failed with[quiet positions handed over] Expecting size of: [MongoResumeTokenCheckpoint[…]] to be greater than 1 but was 1.[warnings that the resume token can't be read] Expecting empty but was: [[WARN] … (java.lang.AssertionError)…].Three tests pass on the previous version too, so they catch a later regression and prove nothing about this change:
an_event_written_after_the_driver_opened_the_change_stream_again_is_delivered_exactly_once, and the two tests ofReactorMongoSubscriptionModelTwoMongosTestnamed above. The reopen test passes on b1e7f34 as well, since b1e7f34 never fell back on a failed token read. It fails only on the change that falls back on every failed read.Still open
ReactorDurableSubscriptionModel, after #1179 is merged:
QuietPositionReportingSubscriptions.findIn(..)when the model is made, and remove it onshutdown(). It needs an interval inReactorDurableSubscriptionModelConfig, likesaveQuietPositionEvery(Duration)andneverSaveQuietPosition()on the blocking config, and the starter needs to pass it on.ReactorCatchupSubscriptionModel,ReactorDcbCatchupSubscriptionModelandReactorStreamCatchupSubscriptionModelhave to answercapability(QuietPositionReportingSubscriptions.class)with the model they wrap. On the reactor sidecapability(..)is a plaininstanceofand does not look through a wrapper.storage.save(..)and no write condition, so a quiet save that is late can replace a newer checkpoint. The blocking model has the same window for a store without a condition, and ADR 142 accepts it there, since the stored position then only moves back.cancelSubscription(..)to returnMono<Void>, which the quiet save has to wait for or check.any(), so the save can't be refused. ThePositionWriterA reactor cancel returns a Mono that completes once the stored state is gone #1179 gives each generation of a subscription is what keeps a quiet save from following the delete incancelSubscription(..), since a cancel retires it and a retired writer begins no write.A
CheckpointWriteConditionNotFulfilledExceptionfrom a quiet position handler is retried like any other error from it, and one from an action is retried with the backoff indefinitely, as on main. The blocking models end delivery on that exception, since there the quiet save is conditional on the version of the lease. The reactor stack has no lease, so only a listener of your own can raise it.