fix(driver): send the read preference the caller asked for

[bernhgen]

Problem

MongoCommand hardcodes the $readPreference field it sends to the server:

private Doc $readPreference = Doc.of("mode", "primaryPreferred");

There is no setter and no way to derive it from the caller's read preference, and asMap()
serialises the field into every command.

On a plain replica set this is invisible: the driver picks the node itself in
getReadConnection(ReadPreference), and mongod ignores $readPreference in the command.
Behind a mongos it is decisive — a sharded cluster routes reads by exactly this field. So
on a sharded cluster every read is effectively primaryPreferred, no matter what was
configured: MorphiumConfig.defaultReadPreference and @DefaultReadPreference on an entity
are silently dropped.

Note that on a sharded cluster the read preference cannot take effect through node selection
either: replicaSet is only set from hello.setName (PooledDriver), a mongos does not report
one, so selectReadConnection() takes its standalone branch and never looks at the read
preference. The command field is the only mechanism there.

The practical consequence is stale reads that the application explicitly asked to avoid. With
primaryPreferred, a brief primary loss on a shard makes mongos read from a secondary; one
that has not caught up answers with an older version of a document — or with nothing at all,
which makes Morphium.reread() return null for a document that exists.

Second, smaller issue: Morphium.reread() reads with the default read preference. Its purpose
is to refresh an object with the state that is in the database, so a possibly lagging
secondary is the wrong source — the driver already forces the primary inside transactions and
in the read-after-write window, but not here.

Both verified in 6.2.10 and in develop.

Fix

1. fix(driver): send the read preference the caller asked for

• MongoCommand gets a transient readPreference with getter/setter; the hardcoded
  $readPreference field is gone. asMap() resolves it as: set on the command → the one the
  connection was handed out for → primaryPreferred as before, so the wire output does not
  change when nobody asks for anything.
• ReadPreferenceType.getMode() returns the wire-protocol names; a tag set is emitted as
  tags: [ { … } ].
• MongoConnection gets getEffectiveReadPreference() / setEffectiveReadPreference() as
  default methods (no-op, so third-party implementations keep compiling). PooledDriver,
  SingleMongoConnectDriver and InMemoryDriver stamp the connection they hand out.
• In PooledDriver the PRIMARY-forcing rules (transaction, read-after-write window, in-memory
  backend) move out of getReadConnection() into effectiveReadPreference(), so what the
  connection is marked with is what the read is really performed with.

2. fix(morphium): reread() reads from the primary

Morphium.reread() uses getReadConnection(ReadPreference.primary()).

Tests

• MongoCommandReadPreferenceTest (12) — what ends up in $readPreference: set on the
  command, inherited from the connection, command wins over connection, the unchanged default,
  every type's wire name, tag sets, and both ends of the chain (a connection handed out for a
  read preference carries it into the command; a write connection is marked PRIMARY).
• PooledDriverEffectiveReadPreferenceTest (4) — the effective read preference: requested one
  kept, PRIMARY without a requested one, PRIMARY forced during a transaction, tag set survives.
• RereadReadPreferenceTest (1) — reread() asks for PRIMARY even with a nearest default.

Green alongside these: the driver package (796), CommandAsMapTest, the reread/transaction
tests under -Pinmem, and poppydb's SecondaryReadPreferenceTest.

One thing for the reviewer

poppydb's MongoCommandHandler rejects mode: "primary" reads on a secondary with 13435,
and its comment relies on morphium always sending primaryPreferred. With this change that
assumption no longer holds, so a read against a secondary that explicitly asks for the primary
now gets the error mongod would return too — correct, but a behaviour change worth naming.

[sboesebeck]

Hi Bernhard,

thanks for this one - the analysis is spot on, and the mongos angle is something I had honestly never looked at. Nice catch. I ran the new tests locally (19/19 green, CommandAsMapTest still happy) and then went through the driver side with a fine comb. Came out longer than I expected, but nothing here is big. In order of importance:

1. SingleMongoConnectDriver: the stamp gets lost

getConnection() always hands out a ConnectionWrapper (line 154/174), and that wrapper does not override get/setEffectiveReadPreference(). So the stamp in getReadConnection()/getPrimaryConnection() hits the no-op default of the interface and is gone. readPreferenceAsDoc() sees null and sends primaryPreferred exactly as before - the fix is inert on one of the three drivers, and no test would notice (MongoCommandReadPreferenceTest only uses ConnectionMock and InMemoryDriver). Delegating both methods to getDelegate() fixes it.

But careful, once that works two things surface:

• With ConnectionType.SECONDARY or ANY the driver sits on a secondary on purpose. DriverBase defaults defaultRP to primary(), so a read without an explicit preference goes out as mode: "primary" - and the secondary answers 13435. mongod does that too, it uses $readPreference for the secondaryOk decision (so "mongod ignores it" in the PR text is not quite right). The server selection spec says for exactly this case: on a direct connection to a secondary, send primaryPreferred if the mode is primary. Please do that when connectionType != PRIMARY.
• SingleMongoConnectDriver stamps the raw preference without the PRIMARY-forcing rules (transaction, read-after-write window, inMemoryBackend) that PooledDriver.effectiveReadPreference() has. Inside a transaction that would send mode: nearest to a mongos, which rejects it. All three inputs live in DriverBase anyway - I'd move effectiveReadPreference() down there and call it from both drivers.

2. The test withoutARequestedOneThePrimaryIsUsed

That one pins down that PooledDriver.getDefaultReadPreference() hardcodes PRIMARY and ignores setDefaultReadPreference(). That is a known wart on my list, not behaviour I want to keep - the moment I fix it, your test goes red. Please drop it, or assert against getDefaultReadPreference() instead of the PRIMARY literal.

3. Tags with mode primary

readPreferenceAsDoc() emits tags for every mode, PRIMARY included. MongoDB rejects that ("Only empty tags are allowed with primary mode"), and ReadPreference.addTag() has no type check, so primary() plus a tag was harmless until now and breaks every read after this PR. Skip the tags when the type is PRIMARY.

4. Failover window - a behaviour change we should know about

Reads with mode: "primary" (PooledDriver default, transactions, read-after-write window, every PoppyDB read, reread()) that hit the ex-primary right after a step-down now get 13435 instead of a silent, possibly stale read. primaryNode stays stale until the next heartbeat (1s), and the read path has no retry for that code - only WriteMongoCommand.isStepDownError() knows it. I think the error is the right answer, but it is new. I'll run the mongodb_rs and poppydb_rs phases on the testrunner once the rest is in and we see what falls out. If it gets noisy, a read-side counterpart to isStepDownError() is probably the way.

Related: on a reconnect after a network error, ReadMongoCommand re-borrows with getDefaultReadPreference() (= primary), not with what the query asked for. Behind a mongos the retry is then routed differently than the first attempt. Cheap fix: put the requested preference on the command (setReadPreference() - you added it, nobody calls it yet) and re-borrow with that.

5. The default is nearest()

DriverSettings ships with nearest(). On a replica set that was already effective through node selection, on a mongos it was always primaryPreferred - and now becomes nearest. So an unconfigured sharded deployment silently loses read-your-own-writes after a plain store(). That is our default, not your bug, but now that the field is wire-effective it is the wrong one. Please switch the default in DriverSettings to primaryPreferred() in this PR - that keeps mongos exactly where it was, and on a replica set it means unconfigured setups read from the primary, like the official drivers do. Needs a ### Changed entry in the CHANGELOG, since RS users who relied on the implicit nearest will notice the load shift.

6. Small stuff

• CHANGELOG entry under [Unreleased] / ### Fixed please, with the why (mongos routing) - that is how we do it here. Same place for reread() now always reading from the primary (and throwing after serverSelectionTimeout if none is reachable), and for the PoppyDB 13435 change you already described in the PR.
• The comment in PoppyDB's MongoCommandHandler (around line 1154) and the javadoc of SecondaryReadPreferenceTest still claim morphium always sends primaryPreferred. You mentioned it yourself, please just update both.
• effectiveReadPreference() normalises rp == null but not rp.getType() == null, and selectReadConnection() switches on the type. new ReadPreference() is public and DriverSettings tolerates a null type, so that is an NPE waiting. Pre-existing hole, but you're standing right next to it - a one-liner.
• fromMap()/asMap() still skip a field named readPreference by name; with @Transient that is redundant, and the Javadoc says "(by name)". Your call.
• The primaryPreferred() fallback in effectiveReadPreference() cannot be reached (getDefaultReadPreference() never returns null) and encodes a different default than the documented primary. Same for the InMemoryDriver ternary - getDefaultReadPreference() there always returns null.

That's it. Fix 1 to 3, flip the default, add the changelog, then I kick off the testrunner and we take it from there.

Viele Grüße,

Stephan

[sboesebeck]

Follow-up on point 4, now with a CI data point.

The first CI run of this PR went red on FastResyncTest.fallbackOnSystemVersionDivergence (poppydb module), develop was green on the same base commit three hours earlier. What happens in the log: node1 is shut down for the failover, node3's ReplicationManager keeps reading from it for a moment, and since the reads now carry mode: "primary", the stepped-down node answers 13435 - periodic index sync, replication loop, reconnect, again. Before the PR those reads were served silently.

I ran the test locally on both branches:

• PR branch, run 1: green, 870 log lines with 13435
• PR branch, run 2: green, 0
• develop: green, 0

So the error storm is caused by the PR (by design - the stale ex-leader is the wrong sync source, and the error is the right answer), and the assertion failure on top of it is timing: with the election churn on the CI box the test fetches the ReplicationManager a second time after its poll and may get a fresh instance whose clear-counter is still 0. The CI rerun of the same commit went green, which fits.

Nothing more to change in this PR for that. Two things I'll take on our side, separate from your PR: the ReplicationManager should treat 13435 as "sync source is no longer primary" and wait for the leadership change instead of retry-looping, and the test needs to hold on to the instance it polled. I'll open issues for both once your revision is in and we've seen the testrunner phases.