PERL-787: Array Update Improvements

[TBSliver]

Includes update to CRUD spec test files

[coveralls]

Coverage decreased (-0.04%) to 83.786% when pulling cde515a on shadow-dot-cat:TBSliver/PERL-787 into e477f53 on mongodb:master.

[xdg]

The spec says there are various situations requiring errors: if the server is less than 3.6 or if it's an unacknowledged write with an opcode. You can mirror the "collation" logic in lines 94-101, but you'll need to define "supports_arrayFiltres" in MongoDB::Link as well.

[TBSliver]

The error messages should be in now for that :)

[xdg]

Please pull the "get a version from featureCompatibilityVersion" logic into a helper sub or else just up to the preamble after line 41. I'd like to see the plan skip_all predicate below simplified to something like unless $supports_array_filters.

[TBSliver]

Yep will do! was debating whether to do that but forgot to come back to it after it was working heh

[TBSliver]

Right have updated the file for factoring out the bit from crud_spec, but have run into a slight concept issue with the featureCompatiblityVersion fetching, re. supports_arrayFilters in MongoDB::_Link. The issue here is, to find out if it is supported, you have to run a command against admin db to get the value. Now, fetching this information could be done in MongoDB::_Topology::_update_topology_from_link though the MongoDB::_Link::set_metadata command, however if this is a mongos set, you cannot read that information, as its undefined (see here) which means you cannot check on all servers. That and it doesnt specify if this works on < 3.4 versions mongod (I assume probably not?)

so, if there is a solution to this, then I can see the implementation being to:

• Add supports_arrayFilters in MongoDB::_Link
• Add featureCompatibilityVersion in MongoDB::_Server
• Add setting supports_arrayFilters in MongoDB::_Link::set_metadata based on featureCompatibilityVersion from server
• fetch featureCompatibilityVersion in MongoDB::_Topology::_update_topology_from_link and propogate through to link via the new server created there.

Of course I may have misunderstood some part of this, but as an _Server instance can have a type of mongos this makes me wonder how this feature is meant to be checked without just running it against the server....

[xdg]

I think there are two levels to your question.

1. How does the driver determine if arrayFilters are supported in MongoDB::Op::_Update? In this case, the determination should be made only based on accepts_wire_version(6). If that is true, then we say we support arrayFilters (and it's up to users not to send it if they are still on FCV 3.4 on a 3.6 server).

2. How does the test know whether to test arrayFilters or skip? Here we're "allowed" to look at the FCV, but as you saw, it's brittle. However, "secretly" I can tell you that if the server is version 3.6, you can do $client->send_admin_command([startSession=>1]). That should succeed on 3.6 if FCV is 3.6 and should fail if the FCV is 3.4.

What the heck is FCV, perhaps you're asking? :-) It's a flag to tell whether the data files found during database startup are in the current server version format or in the prior server version format. It's an internal indicator to help with upgrade/downgrade. In the 3.8, it's possible that external access to FCV will be removed so that users don't even try to rely on it (as we were doing for testing).

[TBSliver]

Yea i figured FCV was used for something like that :)

As for using the send_admin_command -> startSession, that seems to indicate to me that this should wait until PERL-799 (#149) is finished as that seems to be the way (according to the spec) of figuring out if the server supports sessions, instead of implementing two things that check if sessions work, one of which actually causes an exception (albeit in testing, but still, I tend to use the tests for 'how the hell do I do this thing?' a lot of the time haha). either that or put a warning of 'DONT DO THIS' next to the code in t/lib/MongoDBTest.pm, but that again seems like a bad idea in the long run heh

[xdg]

Looks good. I'll squash and merge and we can fix up the test skipping later.