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

Subdoc Fetch can cause panic (xattrs)

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 6.5.0, 6.0.0, 5.5.2
    • 6.5.0, 5.5.3, 6.0.1
    • query
    • None
    • Untriaged
    • Unknown

    Description

      KV Fetch of XATTRS that contains non existent key (or deleted between indexscan and fetch).
      Repro: USE KEYS (need more than 1 key trigger issue, for one key it goes in different code path)

      SELECT META().xattrs._sync
      FROM default USE KEYS ["xxxx", "yy"];
      

      request text:
      <ud>select META().xattrs._sync from default use keys ["xxxx", "yy"];</ud>
       
      stack:
      goroutine 245 [running]:
      github.com/couchbase/query/execution.(*Context).Recover(0xc420733c80)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/context.go:519 +0xbc
      panic(0x4ad5a40, 0x5444380)
      	/usr/local/Cellar/go/1.8.3/libexec/src/runtime/panic.go:489 +0x2cf
      github.com/couchbase/query/datastore/couchbase.getSubDocFetchResults(0xc420d3e7e9, 0x2, 0xc420c13560, 0xc420cdd840, 0x2, 0x2, 0x1, 0xc420cdd840, 0x2)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1123 +0xc03
      github.com/couchbase/query/datastore/couchbase.(*keyspace).Fetch(0xc4204d4460, 0xc420d63c00, 0x2, 0x40, 0xc420d0ea20, 0x53cb140, 0xc420733c80, 0xc420d3e990, 0x1, 0x2, ...)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1072 +0x42e
      github.com/couchbase/query/execution.(*Fetch).flushBatch(0xc420d306c0, 0xc420733c80, 0x10000000474d900)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:140 +0x290
      github.com/couchbase/query/execution.(*Fetch).afterItems(0xc420d306c0, 0xc420733c80)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:82 +0x35
      github.com/couchbase/query/execution.(*base).runConsumer.func1()
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:564 +0x296
      github.com/couchbase/query/util.(*Once).Do(0xc420d307c8, 0xc420add738)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/util/sync.go:53 +0x68
      github.com/couchbase/query/execution.(*base).runConsumer(0xc420d306c0, 0x53c8760, 0xc420d306c0, 0xc420733c80, 0x0, 0x0)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:565 +0xaf
      github.com/couchbase/query/execution.(*Fetch).RunOnce(0xc420d306c0, 0xc420733c80, 0x0, 0x0)
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:66 +0x5c
      created by github.com/couchbase/query/execution.(*base).runConsumer.func1
      	/Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:550 +0x2f6
      

      Attachments

        For Gerrit Dashboard: MB-32120
        # Subject Branch Project Status CR V

        Activity

          Sitaram.Vemulapalli Sitaram Vemulapalli created issue -
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Field Original Value New Value
          Link This issue is triggering CBSE-6021 [ CBSE-6021 ]
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Link This issue is triggering CBSE-6021 [ CBSE-6021 ]
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Link This issue causes CBSE-6021 [ CBSE-6021 ]
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Description KV Fetch of XATTRS that contains non existent key (or deleted before fetch).
          Repro: USE KEYS (need more than 1 key tigger issue)

          {code:java}
          SELECT META().xattrs._sync
          FROM default USE KEYS ["xxxx", "yy"];
          {code}



          {code:java}
          request text:
          <ud>select META().xattrs._sync from default use keys ["xxxx", "yy"];</ud>

          stack:
          goroutine 245 [running]:
          github.com/couchbase/query/execution.(*Context).Recover(0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/context.go:519 +0xbc
          panic(0x4ad5a40, 0x5444380)
          /usr/local/Cellar/go/1.8.3/libexec/src/runtime/panic.go:489 +0x2cf
          github.com/couchbase/query/datastore/couchbase.getSubDocFetchResults(0xc420d3e7e9, 0x2, 0xc420c13560, 0xc420cdd840, 0x2, 0x2, 0x1, 0xc420cdd840, 0x2)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1123 +0xc03
          github.com/couchbase/query/datastore/couchbase.(*keyspace).Fetch(0xc4204d4460, 0xc420d63c00, 0x2, 0x40, 0xc420d0ea20, 0x53cb140, 0xc420733c80, 0xc420d3e990, 0x1, 0x2, ...)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1072 +0x42e
          github.com/couchbase/query/execution.(*Fetch).flushBatch(0xc420d306c0, 0xc420733c80, 0x10000000474d900)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:140 +0x290
          github.com/couchbase/query/execution.(*Fetch).afterItems(0xc420d306c0, 0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:82 +0x35
          github.com/couchbase/query/execution.(*base).runConsumer.func1()
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:564 +0x296
          github.com/couchbase/query/util.(*Once).Do(0xc420d307c8, 0xc420add738)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/util/sync.go:53 +0x68
          github.com/couchbase/query/execution.(*base).runConsumer(0xc420d306c0, 0x53c8760, 0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:565 +0xaf
          github.com/couchbase/query/execution.(*Fetch).RunOnce(0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:66 +0x5c
          created by github.com/couchbase/query/execution.(*base).runConsumer.func1
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:550 +0x2f6
          {code}

          KV Fetch of XATTRS that contains non existent key (or deleted between indexscan and fetch).
          Repro: USE KEYS (need more than 1 key tigger issue)

          {code:java}
          SELECT META().xattrs._sync
          FROM default USE KEYS ["xxxx", "yy"];
          {code}



          {code:java}
          request text:
          <ud>select META().xattrs._sync from default use keys ["xxxx", "yy"];</ud>

          stack:
          goroutine 245 [running]:
          github.com/couchbase/query/execution.(*Context).Recover(0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/context.go:519 +0xbc
          panic(0x4ad5a40, 0x5444380)
          /usr/local/Cellar/go/1.8.3/libexec/src/runtime/panic.go:489 +0x2cf
          github.com/couchbase/query/datastore/couchbase.getSubDocFetchResults(0xc420d3e7e9, 0x2, 0xc420c13560, 0xc420cdd840, 0x2, 0x2, 0x1, 0xc420cdd840, 0x2)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1123 +0xc03
          github.com/couchbase/query/datastore/couchbase.(*keyspace).Fetch(0xc4204d4460, 0xc420d63c00, 0x2, 0x40, 0xc420d0ea20, 0x53cb140, 0xc420733c80, 0xc420d3e990, 0x1, 0x2, ...)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1072 +0x42e
          github.com/couchbase/query/execution.(*Fetch).flushBatch(0xc420d306c0, 0xc420733c80, 0x10000000474d900)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:140 +0x290
          github.com/couchbase/query/execution.(*Fetch).afterItems(0xc420d306c0, 0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:82 +0x35
          github.com/couchbase/query/execution.(*base).runConsumer.func1()
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:564 +0x296
          github.com/couchbase/query/util.(*Once).Do(0xc420d307c8, 0xc420add738)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/util/sync.go:53 +0x68
          github.com/couchbase/query/execution.(*base).runConsumer(0xc420d306c0, 0x53c8760, 0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:565 +0xaf
          github.com/couchbase/query/execution.(*Fetch).RunOnce(0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:66 +0x5c
          created by github.com/couchbase/query/execution.(*base).runConsumer.func1
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:550 +0x2f6
          {code}

          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Description KV Fetch of XATTRS that contains non existent key (or deleted between indexscan and fetch).
          Repro: USE KEYS (need more than 1 key tigger issue)

          {code:java}
          SELECT META().xattrs._sync
          FROM default USE KEYS ["xxxx", "yy"];
          {code}



          {code:java}
          request text:
          <ud>select META().xattrs._sync from default use keys ["xxxx", "yy"];</ud>

          stack:
          goroutine 245 [running]:
          github.com/couchbase/query/execution.(*Context).Recover(0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/context.go:519 +0xbc
          panic(0x4ad5a40, 0x5444380)
          /usr/local/Cellar/go/1.8.3/libexec/src/runtime/panic.go:489 +0x2cf
          github.com/couchbase/query/datastore/couchbase.getSubDocFetchResults(0xc420d3e7e9, 0x2, 0xc420c13560, 0xc420cdd840, 0x2, 0x2, 0x1, 0xc420cdd840, 0x2)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1123 +0xc03
          github.com/couchbase/query/datastore/couchbase.(*keyspace).Fetch(0xc4204d4460, 0xc420d63c00, 0x2, 0x40, 0xc420d0ea20, 0x53cb140, 0xc420733c80, 0xc420d3e990, 0x1, 0x2, ...)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1072 +0x42e
          github.com/couchbase/query/execution.(*Fetch).flushBatch(0xc420d306c0, 0xc420733c80, 0x10000000474d900)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:140 +0x290
          github.com/couchbase/query/execution.(*Fetch).afterItems(0xc420d306c0, 0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:82 +0x35
          github.com/couchbase/query/execution.(*base).runConsumer.func1()
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:564 +0x296
          github.com/couchbase/query/util.(*Once).Do(0xc420d307c8, 0xc420add738)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/util/sync.go:53 +0x68
          github.com/couchbase/query/execution.(*base).runConsumer(0xc420d306c0, 0x53c8760, 0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:565 +0xaf
          github.com/couchbase/query/execution.(*Fetch).RunOnce(0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:66 +0x5c
          created by github.com/couchbase/query/execution.(*base).runConsumer.func1
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:550 +0x2f6
          {code}

          KV Fetch of XATTRS that contains non existent key (or deleted between indexscan and fetch).
          Repro: USE KEYS (need more than 1 key trigger issue, for one key it goes in different code path)

          {code:java}
          SELECT META().xattrs._sync
          FROM default USE KEYS ["xxxx", "yy"];
          {code}



          {code:java}
          request text:
          <ud>select META().xattrs._sync from default use keys ["xxxx", "yy"];</ud>

          stack:
          goroutine 245 [running]:
          github.com/couchbase/query/execution.(*Context).Recover(0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/context.go:519 +0xbc
          panic(0x4ad5a40, 0x5444380)
          /usr/local/Cellar/go/1.8.3/libexec/src/runtime/panic.go:489 +0x2cf
          github.com/couchbase/query/datastore/couchbase.getSubDocFetchResults(0xc420d3e7e9, 0x2, 0xc420c13560, 0xc420cdd840, 0x2, 0x2, 0x1, 0xc420cdd840, 0x2)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1123 +0xc03
          github.com/couchbase/query/datastore/couchbase.(*keyspace).Fetch(0xc4204d4460, 0xc420d63c00, 0x2, 0x40, 0xc420d0ea20, 0x53cb140, 0xc420733c80, 0xc420d3e990, 0x1, 0x2, ...)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/datastore/couchbase/couchbase.go:1072 +0x42e
          github.com/couchbase/query/execution.(*Fetch).flushBatch(0xc420d306c0, 0xc420733c80, 0x10000000474d900)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:140 +0x290
          github.com/couchbase/query/execution.(*Fetch).afterItems(0xc420d306c0, 0xc420733c80)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:82 +0x35
          github.com/couchbase/query/execution.(*base).runConsumer.func1()
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:564 +0x296
          github.com/couchbase/query/util.(*Once).Do(0xc420d307c8, 0xc420add738)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/util/sync.go:53 +0x68
          github.com/couchbase/query/execution.(*base).runConsumer(0xc420d306c0, 0x53c8760, 0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:565 +0xaf
          github.com/couchbase/query/execution.(*Fetch).RunOnce(0xc420d306c0, 0xc420733c80, 0x0, 0x0)
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/fetch.go:66 +0x5c
          created by github.com/couchbase/query/execution.(*base).runConsumer.func1
          /Users/sitaram/master/olap/src/github.com/couchbase/query/execution/base.go:550 +0x2f6
          {code}

          Build couchbase-server-6.5.0-1708 contains gomemcached commit 5125a94 with commit message:
          MB-32120. Handle KEY_ENOENT for Subdoc

          build-team Couchbase Build Team added a comment - Build couchbase-server-6.5.0-1708 contains gomemcached commit 5125a94 with commit message: MB-32120 . Handle KEY_ENOENT for Subdoc

          Is this issue different from MB-31990/MB-30467 ?

          mihir.kamdar Mihir Kamdar (Inactive) added a comment - Is this issue different from MB-31990 / MB-30467 ?
          wayne Wayne Siu made changes -
          Link This issue blocks MB-31456 [ MB-31456 ]
          wayne Wayne Siu made changes -
          Link This issue blocks MB-31466 [ MB-31466 ]

          Build couchbase-server-5.5.3-4034 contains gomemcached commit e05ec35 with commit message:
          MB-32120. Handle KEY_ENOENT for Subdoc

          build-team Couchbase Build Team added a comment - Build couchbase-server-5.5.3-4034 contains gomemcached commit e05ec35 with commit message: MB-32120 . Handle KEY_ENOENT for Subdoc

          Build couchbase-server-6.0.1-1974 contains gomemcached commit e05ec35 with commit message:
          MB-32120. Handle KEY_ENOENT for Subdoc

          build-team Couchbase Build Team added a comment - Build couchbase-server-6.0.1-1974 contains gomemcached commit e05ec35 with commit message: MB-32120 . Handle KEY_ENOENT for Subdoc
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Assignee Keshav Murthy [ keshav ] Mihir Kamdar [ mihir.kamdar ]
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Closed [ 6 ]
          Sitaram.Vemulapalli Sitaram.Vemulapalli made changes -
          Actual End 2018-11-26 16:09 (issue has been closed)
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Resolved [ 5 ]
          Sitaram.Vemulapalli Sitaram.Vemulapalli made changes -
          Actual End 2018-11-26 16:09 2018-11-26 16:09 (issue has been resolved)
          adamf Adam Fraser added a comment -

          Mobile has several scenarios where index entries are expected to be associated with deleted documents.  This is because system xattrs are retained when a document is deleted, and mobile still needs the ability to query those documents.  Those indexes are created WITH {"retain_deleted_xattr":true}.

          To fetch those documents successfully, the sub-document doc flag AccessDeleted would need to be set for the subdoc operation. (https://github.com/couchbase/kv_engine/blob/a8f0097b10edfe2a86ec9183b5fc303fb8082aba/include/memcached/protocol_binary.h#L312)

          There are three Sync Gateway indexes that create the index with retain_deleted_xattr:true and are also queried with non-covering queries: sg_access, sg_roleAccess, sg_allDocs. 

          Sitaram Vemulapalli How does query currently handle document in this state when doing a fetch?  Is the index row just ignored in the resultset, or does the fetch successfully retrieve the xattr associated with the tombstoned document?  Let me know, and I'll review the mobile use cases in more detail to determine whether it's strictly necessary for us to retrieve the xattr in the non-covering query case.

           

           

          adamf Adam Fraser added a comment - Mobile has several scenarios where index entries are expected to be associated with deleted documents.  This is because system xattrs are retained when a document is deleted, and mobile still needs the ability to query those documents.  Those indexes are created WITH {"retain_deleted_xattr":true}. To fetch those documents successfully, the sub-document doc flag AccessDeleted would need to be set for the subdoc operation. ( https://github.com/couchbase/kv_engine/blob/a8f0097b10edfe2a86ec9183b5fc303fb8082aba/include/memcached/protocol_binary.h#L312) There are three Sync Gateway indexes that create the index with retain_deleted_xattr:true and are also queried with non-covering queries: sg_access, sg_roleAccess, sg_allDocs.  Sitaram Vemulapalli How does query currently handle document in this state when doing a fetch?  Is the index row just ignored in the resultset, or does the fetch successfully retrieve the xattr associated with the tombstoned document?  Let me know, and I'll review the mobile use cases in more detail to determine whether it's strictly necessary for us to retrieve the xattr in the non-covering query case.    
          korrigan.clark Korrigan Clark (Inactive) made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

          You should review the spec which mentioned all these.

          This what N1QL does for xattrs KV Fetch.

          • Given document key it uses SubDoc API to Fetch Xattrs + Whole document with deleted=true
          • Then METTA().xattrs. will be filed and virtual attribute will have deleted info, Whole document also present. If document deleted whole document value will be NULL
          • SELECT META(d).xattrs._sync, META(d).xattrs.`$document`, d FROM default AS d USE KEYS "k1";
          • If k1 is deleted d will be NULL
          • If you can filter out what ever you want
          Sitaram.Vemulapalli Sitaram Vemulapalli added a comment - You should review the spec which mentioned all these. This what N1QL does for xattrs KV Fetch. Given document key it uses SubDoc API to Fetch Xattrs + Whole document with deleted=true Then METTA().xattrs. will be filed and virtual attribute will have deleted info, Whole document also present. If document deleted whole document value will be NULL SELECT META(d).xattrs._sync, META(d).xattrs.`$document`, d FROM default AS d USE KEYS "k1"; If k1 is deleted d will be NULL If you can filter out what ever you want
          adamf Adam Fraser added a comment -

          Sitaram Vemulapalli Thanks for the link to the spec - that looks like the expected handling from Mobile's perspective.  Given that, can you confirm that the cases of non-existent keys addressed by this ticket would be purged documents (i.e. both document and xattr no longer exist)? 

          adamf Adam Fraser added a comment - Sitaram Vemulapalli Thanks for the link to the spec - that looks like the expected handling from Mobile's perspective.  Given that, can you confirm that the cases of non-existent keys addressed by this ticket would be purged documents (i.e. both document and xattr no longer exist)? 
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Closed [ 6 ]
          Sitaram.Vemulapalli Sitaram Vemulapalli made changes -
          Link This issue is triggering CBSE-6268 [ CBSE-6268 ]

          Build sync_gateway-2.7.0-36 contains gomemcached commit 5125a94 with commit message:
          MB-32120. Handle KEY_ENOENT for Subdoc

          build-team Couchbase Build Team added a comment - Build sync_gateway-2.7.0-36 contains gomemcached commit 5125a94 with commit message: MB-32120 . Handle KEY_ENOENT for Subdoc

          People

            mihir.kamdar Mihir Kamdar (Inactive)
            Sitaram.Vemulapalli Sitaram Vemulapalli
            Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty