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

Move decrement of vBucket doc count on collection drop to flush rather than erase

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Won't Do
    • Major
    • None
    • 7.1.0
    • couchbase-bucket
    • None

    Description

      See MB-50061 and MB-42272 for past thoughts/commentary. Moving the docCount decrement by the collection size to flush rather than the collection erasure would allow us to simplify doc counting for magma and use common logic between magma and couchstore.

      Probably the biggest issue here is what we do with the persisted vBucket docCount. Naively we would just decrement this on the flush of the drop, and that is easily workable for magma as it's a) maintained by kv_engine and b) does not have to deal with any upgrade case. Couchstore is the harder problem though; the count is maintained in couchstore so we'd need to decrement it manually somehow on the flush of collection drops and also prevent couchstore from decrementing it for logically deleted items during compaction. Alternatively we could track a logically deleted item count which is incremented on collection drop and decremented on collection erasure. We could then use that item count to correct the persisted docCount.

      Upgrade is another issue as we could have a pre-upgrade state of a dropped but not yet purged collection and the item count stored by couchstore would be incorrect in the post-upgrade world. We don't track stats for dropped collections for couchstore at the moment so the only way to work out how much to decrement our doc count by would be to run a full compaction and decrement it for logically deleted item as we currently do in 7.0. I suppose something like that would probably want to go in an upgrade script if we were to try to solve the problem in this way. We might be able to work around this if we tracked a logically deleted item count. Should we purge logically deleted items when the count is 0 we could just decrement the persisted docCount instead.

      Post-Neo an upgrade for magma may require some separate script but with the current MagmaKVStore code we should be able to just read and write UserStats with the desired values and skip the full compaction.

      Considering the complexities of changing the couchstore implementation it is probably not going to be a net gain in terms of reducing code complexity.

      I don't think that the code is too bad when it comes to making this change for magma. Complexities are added in some places but we can remove some others and I think that it's a fairly even tradeoff if we don't mind the difference in behaviour between magma and couchstore. One question is what should we do for magma with respect to full and value eviction. Value eviction counts from the HashTable so should we track an extra logically deleted counter during flushes and warmup to make it behave the same as full eviction (extra complexity) or just change the behaviour for magma full eviction? Magma value eviction feels semi-pointless anyway so not sure it's worth adding any value eviction specific change.

      Taking a step back, half the reason for doing this was to make code simpler/behaviours more consistent between the two KVStores. I think there's no point making a change only for magma buckets here as we end up with something about as a complicated as what we have now but a less consistent behaviour (magma currently is only inconsistent in the even of a collection drop during quorum failover - MB-50061). Given that making this change for couchstore adds more/different complexity into item counting as couchstore manages the doc count rather than kv_engine I don't think that this change makes things any better than they currently are.

      Attachments

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

        Activity

          People

            ben.huddleston Ben Huddleston
            ben.huddleston Ben Huddleston
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty