[SERVER-67391] Potentially dead code, try - catch block where exception wont be thrown Created: 20/Jun/22  Updated: 27/Oct/23  Resolved: 03/Oct/23

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

Type: Improvement Priority: Major - P3
Reporter: Sulabh Mahajan Assignee: David Dominguez Sal
Resolution: Gone away Votes: 0
Labels: neweng, techdebt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
Assigned Teams:
Storage Execution EMEA
Sprint: Execution EMEA Team 2023-10-16
Participants:

 Description   

The following code gets a lock before collecting the slow operation statistics from the storage engine:

            try {
                // Retrieving storage stats should not be blocked by oplog application.
                ShouldNotConflictWithSecondaryBatchApplicationBlock shouldNotConflictBlock(
                    opCtx->lockState());
                Lock::GlobalLock lk(opCtx,
                                    MODE_IS,
                                    Date_t::now() + Milliseconds(500),
                                    Lock::InterruptBehavior::kLeaveUnlocked);
                if (lk.isLocked()) {
                    _debug.storageStats = opCtx->recoveryUnit()->getOperationStatistics();
                } else {
                    LOGV2_WARNING_OPTIONS(
                        20525,
                        {component},
                        "Failed to gather storage statistics for {opId} due to {reason}",
                        "Failed to gather storage statistics for slow operation",
                        "opId"_attr = opCtx->getOpID(),
                        "error"_attr = "lock acquire timeout"_sd);
                }
            } catch (const ExceptionForCat<ErrorCategory::Interruption>& ex) {
                LOGV2_WARNING_OPTIONS(
                    20526,
                    {component},
                    "Failed to gather storage statistics for {opId} due to {reason}",
                    "Failed to gather storage statistics for slow operation",
                    "opId"_attr = opCtx->getOpID(),
                    "error"_attr = redact(ex));
            }
        }

The interrupt behaviour for the lock is Lock::InterruptBehavior::kLeaveUnlocked, which doesn't seem to throw an exception:

    /**
     * The interrupt behavior is used to tell a lock how to handle an interrupted lock acquisition.
     */
    enum class InterruptBehavior {
        kThrow,         // Throw the interruption exception.
        kLeaveUnlocked  // Suppress the exception, but leave unlocked such that a call to isLocked()
                        // returns false.
    };

But in the code, we try to catch the exception of type interrupt and return a different error message than when the lock times out vs is interrupted. It is likely that the catch block is not needed and the code will never throw an exception.



 Comments   
Comment by David Dominguez Sal [ 03/Oct/23 ]

In the master branch, the lock applies the  Lock::InterruptBehavior::kThrow behavior. So, the code may throw an exception. The resolution is to keep the catch code.

Generated at Thu Feb 08 06:08:01 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.