Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-47749

handleIndexMergeSnapshot() panic from extraneous s.muSnap.Unlock() calls

    XMLWordPrintable

Details

    • Untriaged
    • 1
    • Unknown

    Description

      secondary/indexer/storage_manager.go handleIndexMergeSnapshot() does not lock, and its caller does not hold, the s.muSnap mutex (apparently it used to lock this at the top), but it still contains rarely-entered error reporting code blocks that unlock this mutex. If one of these error blocks gets entered, it triggers a panic.

      The fix is to delete the s.muSnap.Unlock() calls from this method. There are three of them (one is in commented-out code and two are in live code).
       

      Seen in currently undelivered new version of set14_rebalance_test.go TestFailoverAndRebalance:
      http://ci2i-unstable.northscale.in/gsi-04.08.2021-09.40.fail.html

      fatal error: sync: unlock of unlocked mutex
       
      goroutine 166 [running]:
      runtime.throw(0x131592e, 0x1e)
              /home/buildbot/.cbdepscache/exploded/x86_64/go-1.16.5/go/src/runtime/panic.go:1117 +0x72 fp=0xc006507ba0 sp=0xc006507b70 pc=0x43fdd2
      sync.throw(0x131592e, 0x1e)
              /home/buildbot/.cbdepscache/exploded/x86_64/go-1.16.5/go/src/runtime/panic.go:1103 +0x35 fp=0xc006507bc0 sp=0xc006507ba0 pc=0x474375
      sync.(*Mutex).unlockSlow(0xc004e96610, 0xffffffff)
              /home/buildbot/.cbdepscache/exploded/x86_64/go-1.16.5/go/src/sync/mutex.go:196 +0xd8 fp=0xc006507be8 sp=0xc006507bc0 pc=0x491e18
       
       
      Unlock call that panicked:
       
      sync.(*Mutex).Unlock(...)
              /home/buildbot/.cbdepscache/exploded/x86_64/go-1.16.5/go/src/sync/mutex.go:190
       
       
      Caused by s.muSnap.Unlock() in handleIndexMergeSnapshot (storage_manager.go:1720):
       
      github.com/couchbase/indexing/secondary/indexer.(*storageMgr).handleIndexMergeSnapshot(0xc004e96580, 0x14a4a80, 0xc00cdd0ff0)
              /opt/build/goproj/src/github.com/couchbase/indexing/secondary/indexer/storage_manager.go:1720 +0x699 fp=0xc006507f20 sp=0xc006507be8 pc=0xfb67f9
       
       
      github.com/couchbase/indexing/secondary/indexer.(*storageMgr).handleSupvervisorCommands(0xc004e96580, 0x14a4a80, 0xc00cdd0ff0)
              /opt/build/goproj/src/github.com/couchbase/indexing/secondary/indexer/storage_manager.go:224 +0x1b2 fp=0xc006507f58 sp=0xc006507f20 pc=0xfab8f2
      github.com/couchbase/indexing/secondary/indexer.(*storageMgr).run(0xc004e96580)
              /opt/build/goproj/src/github.com/couchbase/indexing/secondary/indexer/storage_manager.go:182 +0x48 fp=0xc006507fd8 sp=0xc006507f58 pc=0xfab628
      runtime.goexit()
              /home/buildbot/.cbdepscache/exploded/x86_64/go-1.16.5/go/src/runtime/asm_amd64.s:1371 +0x1 fp=0xc006507fe0 sp=0xc006507fd8 pc=0x4792e1
      created by github.com/couchbase/indexing/secondary/indexer.NewStorageManager
              /opt/build/goproj/src/github.com/couchbase/indexing/secondary/indexer/storage_manager.go:157 +0x2e5
      

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            Build couchbase-server-7.1.0-1124 contains indexing commit 23371d9 with commit message:
            MB-47749 Delete s.muSnap.Unlock() calls from handleIndexMergeSnapshot()

            build-team Couchbase Build Team added a comment - Build couchbase-server-7.1.0-1124 contains indexing commit 23371d9 with commit message: MB-47749 Delete s.muSnap.Unlock() calls from handleIndexMergeSnapshot()

             Kevin Cherkauer, can you please provide steps to validate?

            hemant.rajput Hemant Rajput added a comment -   Kevin Cherkauer , can you please provide steps to validate?

            Hemant Rajput I think this one needs to be dev verified as it was found and fixed through code inspection, not from actually hitting the bugs.

            kevin.cherkauer Kevin Cherkauer (Inactive) added a comment - Hemant Rajput I think this one needs to be dev verified as it was found and fixed through code inspection, not from actually hitting the bugs.

            Kevin Cherkauer, I've marked the ticket for Dev-verification. Please close the ticket as per your convenience. 

            hemant.rajput Hemant Rajput added a comment - Kevin Cherkauer , I've marked the ticket for Dev-verification. Please close the ticket as per your convenience. 

            Dev verifying per QE request.

            kevin.cherkauer Kevin Cherkauer (Inactive) added a comment - Dev verifying per QE request.

            People

              kevin.cherkauer Kevin Cherkauer (Inactive)
              kevin.cherkauer Kevin Cherkauer (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty