[SERVER-24732] add a runBatchWriteCommand() method to Shard that checks for errors by unwrapping the BatchedCommandResponse Created: 22/Jun/16 Updated: 19/Jul/16 Resolved: 12/Jul/16 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | 3.3.8 |
| Fix Version/s: | 3.3.10 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Esha Maharishi (Inactive) | Assignee: | Jess Fan |
| Resolution: | Done | Votes: | 0 |
| Labels: | neweng | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||
| Sprint: | Sharding 17 (07/15/16) | ||||||||||||
| Participants: | |||||||||||||
| Description |
|
The current Shard::runCommand() method only checks for the request status, command status, and write concern error. Since Shard::runCommand simply takes a BSONObj cmdObj, it's dangerously easy to pass in a BatchedCommandRequest in BSONObj form and assume that retries and error checking will be correctly handled.
|
| Comments |
| Comment by Githook User [ 12/Jul/16 ] |
|
Author: {u'name': u'Jess Fan', u'email': u'jess.fan@10gen.com'}Message: |
| Comment by Spencer Brody (Inactive) [ 06/Jul/16 ] |
|
As part of this, the new runBatchWriteCommand method on Shard should return BatchedCommandResponse, rather then taking a BatchedCommandResponse* output parameter |
| Comment by Spencer Brody (Inactive) [ 30/Jun/16 ] |
|
We should also make sure that as part of this we start informing the ReplicaSetMonitor of errors in the BatchedCommandReply (a la |
| Comment by Esha Maharishi (Inactive) [ 30/Jun/16 ] |
|
There are a bunch of jstests blacklisted from the sharding_continuous_config_stepdown suite because "[write] commands against the config shard do not support retries yet." ShardingCatalogClient::runBatchWriteCommand does not uses Shard's retry logic, but rather takes a retry policy and does its own retries. It should probably be investigated whether any of those are blacklisted due to assumptions about not retrying writes from mongos. |
| Comment by Esha Maharishi (Inactive) [ 24/Jun/16 ] |
|
ShardingCatalogManager::_processBatchWriteResponse and ShardingCatalogClient::_processBatchWriteResponse (which duplicated each other) were replaced with Shard::CommandResponse::processBatchWriteResponse in This ticket should add a Shard::runBatchWriteCommand that replaces ShardingCatalogClient::_runBatchWriteCommand and uses the new Shard::CommandResponse::processBatchWriteResponse for its retry logic. |