[SERVER-40958] Deprecate the 'scandata' argument in 'validate' Created: 02/May/19  Updated: 29/Oct/23  Resolved: 30/May/19

Status: Closed
Project: Core Server
Component/s: Storage
Affects Version/s: None
Fix Version/s: 4.1.14

Type: Improvement Priority: Minor - P4
Reporter: Gregory Wlodarek Assignee: Gregory Noma
Resolution: Fixed Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Documented
is documented by DOCS-12767 Docs for SERVER-40958: Deprecate the ... Closed
Duplicate
is duplicated by SERVER-40957 Update ValidateCmdLevel enum to use a... Closed
Backwards Compatibility: Fully Compatible
Backport Requested:
v4.0, v3.6
Sprint: Execution Team 2019-06-03
Participants:

 Description   

Today, setting scandata to true, sets the ValidateCmdLevel to kValidateRecordStore, which is not checked against anywhere, and therefore unused. We should log a message saying that this argument is deprecated and remove it in a future release.

Additionally, we can convert the 'validate' command to use IDL.



 Comments   
Comment by Gregory Noma [ 31/May/19 ]

I think it makes sense to simply remove it from the 3.6 -> 4.2 documentation, given that it does nothing in these releases.

Comment by Ravind Kumar (Inactive) [ 31/May/19 ]

Ah, I likely misinterpreted the if/elseif.

In that case, in the short term are there objections to simply removing scandata from 3.6 -> 4.2 documentation, or simply marking it as deprecated w/ no function description? If it wasn't doing anything before, and we've deprecated it moving forward, it seems unlikely to be a breaking change for users utilizing this (again considering this is an internal-facing utility).

Comment by Gregory Noma [ 31/May/19 ]

I don't think that scandata stomps full. If full is set to true then the value of scandata is completely ignored. If full is set to false then the value of scandata is read but never used. Thus I believe that it behaves as expected based on the value of full.

Comment by Ravind Kumar (Inactive) [ 31/May/19 ]

I think so, especially if the option doesn't really do anything. I would argue it might be a bug, since a user could set full : true and then scandata:false and end up doing something unexpected (assuming my reading of the code is correct and setting scandata stomps the value of full. If that's a trivial fixup, at least.

If it's non-trivial, I'd like to at least remove the option so users don't risk specifying it and getting unexpected results. This is an internal-only command, I don't know how often folks are actually using it, but to be safe its an easy fix.

Comment by Gregory Noma [ 31/May/19 ]

ravind.kumar ValidateRecordStore appears to not be checked anywhere since 3.6, thus causing scandata to have no effect. Do you think it would make sense to backport this to 3.6?

Comment by Ravind Kumar (Inactive) [ 31/May/19 ]

gregory.noma question:

Looking at the code prior to 4.2, it looks like specifying validate with scandata would stomp full.  I'm having trouble tracking down exactly where kValidateRecordStore is used and what it implies, but if my rough reading of the code is correct, it looks like I'll need to patch the behavior in 4.0 and possibly earlier.

 

Comment by Githook User [ 30/May/19 ]

Author:

{'email': 'gregory.noma@gmail.com', 'name': 'Gregory Noma', 'username': 'gregorynoma'}

Message: SERVER-40958 Deprecate the 'scandata' argument in 'validate', update corresponding ValidateCmdLevel enum
Branch: master
https://github.com/mongodb/mongo/commit/119101f32b246b883ea255ab6b7bd2cae50599de

Generated at Thu Feb 08 04:56:25 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.