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

Subdoc Fetch can cause panic (xattrs)

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Mad-Hatter, 6.0.0, 5.5.2
    • Fix Version/s: Mad-Hatter, 5.5.3, 6.0.1
    • Component/s: query
    • Labels:
      None
    • Triage:
      Untriaged
    • Is this a Regression?:
      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

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

          Activity

          Hide
          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

          Show
          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
          Hide
          mihir.kamdar Mihir Kamdar added a comment -

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

          Show
          mihir.kamdar Mihir Kamdar added a comment - Is this issue different from MB-31990 / MB-30467 ?
          Hide
          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

          Show
          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
          Hide
          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

          Show
          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
          Hide
          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.

           

           

          Show
          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.    
          Hide
          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
          Show
          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
          Hide
          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)? 

          Show
          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)? 

            People

            • Assignee:
              mihir.kamdar Mihir Kamdar
              Reporter:
              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

                  Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.