[SERVER-34748] Consider other error codes before calling invariantWTOK in WiredTigerSession::getCursor() Created: 30/Apr/18  Updated: 29/Oct/23  Resolved: 08/Jun/18

Status: Closed
Project: Core Server
Component/s: Storage
Affects Version/s: 3.4.14, 3.6.4
Fix Version/s: 4.1.1

Type: Improvement Priority: Major - P3
Reporter: Kyle Suarez Assignee: Ben Judd
Resolution: Fixed Votes: 0
Labels: neweng, nyc
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: Storage NYC 2018-06-04, Storage NYC 2018-06-18
Participants:

 Description   

When obtaining a new cursor, we call WT_SESSION::open_cursor(). We have an invariant that the return status is OK if the error is not ENOENT:

    int ret = _session->open_cursor(
        _session, uri.c_str(), NULL, forRecordStore ? "" : "overwrite=false", &c);
    if (ret != ENOENT)
        invariantWTOK(ret);

If we hit the file limit, this call can also return EMFILE ("Too many open files"). We should consider either also checking for this (or similar) return codes so that we can crash with a friendlier error message, or make an fassertWTOK(), as this situation is not an invariant failure in the sense that we have made some sort of programming error.



 Comments   
Comment by Githook User [ 07/Jun/18 ]

Author:

{'username': 'Icantjuddle', 'name': 'Ben Judd', 'email': 'ben.judd@10gen.com'}

Message: SERVER-34748 Added error code check to wtRCToStatus; provides friendlier return code on EMFILE err
Branch: master
https://github.com/mongodb/mongo/commit/2afa433e9d8a8afcafa31c9a62e80b8ec8953109

Comment by Ben Judd [ 06/Jun/18 ]

Per an in-person conversation with milkie we decided to just do a check for this error in the the invariant WTOK error-code check phase.

Comment by Ben Judd [ 05/Jun/18 ]

I meant moreso in the location considered by this ticket or all locations. In other words should changing the error handling be its own ticket apart from fixing this ticket?

Comment by Eric Milkie [ 05/Jun/18 ]

I would prefer we get rid of invariantWTOK and replace it with fassertWTOK, for this ticket (rather than having both). geert.bosch what do you think?

Comment by Ben Judd [ 05/Jun/18 ]

Is changing all the invarientWTOK to fassertWTOK within the scope of this ticket?

Comment by Eric Milkie [ 04/Jun/18 ]

I am on-board with making an fassertWTOK(), but I would like to convert all the invariantWTOK's to it (and thus treat WiredTiger like a third-party black box library, similar to how the code treats other libraries.)
Also, I've never liked invariantWTOK's behavior that in some circumstances it can throw an exception instead of crashing with an error. I wish there was a way to indicate this behavior in our new fassertWTOK function.

Comment by Ben Judd [ 04/Jun/18 ]

milkie Thoughts? ^

Comment by Kyle Suarez [ 04/Jun/18 ]

If it didn't take too much work, I think a new fassertWTOK() would be nice to have, as long as the Storage Team agrees with the idea. It would be nice if this fatal assertion included a message, but unfortunately fassert() does not yet accept one (SERVER-13608), so it sounds reasonable for this fassertWTOK() to log a message with severe() and then crash.

Comment by Ben Judd [ 04/Jun/18 ]

Just to be clear, are you looking for a new fassertWTOK to be made for this situation as one does not exist or prefer to use appropriate logging and crash?

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