[SERVER-29241] WiredTigerRecordStore::updateRecord has an unnecessary WT_CURSOR.set_key Created: 16/May/17  Updated: 30/Oct/23  Resolved: 21/Jul/17

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

Type: Improvement Priority: Major - P3
Reporter: Keith Bostic (Inactive) Assignee: Keith Bostic (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: Storage 2017-05-29, Storage 2017-06-19, Storage 2017-07-10, Storage 2017-07-31
Participants:

 Description   

WiredTigerRecordStore::updateRecord has an unnecessary WT_CURSOR.set_key.

The key doesn't need to be re-set after the search, and, in fact, setting it here may slow down the subsequent WT_CURSOR.insert operation.

The (untested) change I'm suggesting:

*** wiredtiger_record_store.cpp.orig 2017-05-16 17:25:39.768013595 -0400
--- wiredtiger_record_store.cpp.new   2017-05-16 17:25:45.920785403 -0400
***************
*** 1434,1440 ****
          return {ErrorCodes::IllegalOperation, "Cannot change the size of a doc
ument in the oplog"};
      }
      
-     c->set_key(c, _makeKey(id));
      WiredTigerItem value(data, len);
      c->set_value(c, value.Get());
      ret = WT_OP_CHECK(c->insert(c));
--- 1434,1439 ----



 Comments   
Comment by Githook User [ 21/Jul/17 ]

Author:

{u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith.bostic@mongodb.com'}

Message: SERVER-29241 WiredTigerRecordStore::updateRecord has an unnecessary WT_CURSOR.set_key
Branch: master
https://github.com/mongodb/mongo/commit/301bfb35d7008f60586aeed00dd67087ad4254c0

Comment by Keith Bostic (Inactive) [ 18/Jul/17 ]

The cursor insert/remove pair can't be changed, storage layer cursors are configured with overwrite=false.

Comment by Keith Bostic (Inactive) [ 17/May/17 ]

There are similar patterns in WiredTigerIndexUnique::_unindex and WiredTigerIndexStandard::_unindex:

    auto triggerWriteConflictAtPoint = [&keyItem](WT_CURSOR* point) {
        // WT_NOTFOUND may occur during a background index build. Insert a dummy
        // delete it again to trigger a write conflict in case this is being con
        // indexed by the background indexer.
        point->set_key(point, keyItem.Get());
        point->set_value(point, emptyItem.Get());
        invariantWTOK(WT_OP_CHECK(point->insert(point)));
        point->set_key(point, keyItem.Get());
        invariantWTOK(WT_OP_CHECK(point->remove(point)));
    };

If that's called often enough to be interesting, the second set_key is unnecessary, plus the code path could be optimized by doing a cursor update/remove pair, which would be faster because it avoids a search.

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