Cassandra
  1. Cassandra
  2. CASSANDRA-2503

Eagerly re-write data at read time ("superseding / defragmenting")

    Details

      Description

      Once CASSANDRA-2498 is implemented, it will be possible to implement an optimization to eagerly rewrite ("supersede") data at read time. If a successful read needed to hit more than a certain threshold of sstables, we can eagerly rewrite it in a new sstable, and 2498 will allow only that file to be accessed. This basic approach would improve read performance considerably, but would cause a lot of duplicate data to be written, and would make compaction's work more necessary.

      Augmenting the basic idea, if when we superseded data in a file we marked it as superseded somehow, the next compaction that touched that file could remove the data. Since our file format is immutable, the values that a particular sstable superseded could be recorded in a component of that sstable. If we always supersede at the "block" level (as defined by CASSANDRA-674 or CASSANDRA-47), then the list of superseded blocks could be represented using a generation number and a bitmap of block numbers. Since 2498 would already allow for sstables to be eliminated due to timestamps, this information would probably only be used at compaction time (by loading all superseding information in the system for the sstables that are being compacted).

      Initially described on 1608.

      1. 2503.txt
        2 kB
        Jonathan Ellis
      2. 2503-v2.txt
        2 kB
        Jonathan Ellis
      3. 2503-v3.txt
        5 kB
        Jonathan Ellis

        Issue Links

          Activity

          Hide
          Jonathan Ellis added a comment -

          Straightforward patch attached.

          Show
          Jonathan Ellis added a comment - Straightforward patch attached.
          Hide
          Sylvain Lebresne added a comment -

          On the technical side:

          • we probably should skip the commit log (by using Table.apply(rm, false) directly).
          • what is the reason for limiting this to SizeTieredCompaction?

          On the idea itself, I won't hide that I'm less than enthusiastic. It feels to me like the wrong fix to the 'compaction is behind' problem. This will be basically be triggered when compaction is behind, but is basically solving the problem temporarily by adding more pressure on compaction. I'd really like it if we could benchmark/evaluate this before adding it because I kind of fear there is scenario where it will do more harm than help.

          Show
          Sylvain Lebresne added a comment - On the technical side: we probably should skip the commit log (by using Table.apply(rm, false) directly). what is the reason for limiting this to SizeTieredCompaction? On the idea itself, I won't hide that I'm less than enthusiastic. It feels to me like the wrong fix to the 'compaction is behind' problem. This will be basically be triggered when compaction is behind, but is basically solving the problem temporarily by adding more pressure on compaction. I'd really like it if we could benchmark/evaluate this before adding it because I kind of fear there is scenario where it will do more harm than help.
          Hide
          Jonathan Ellis added a comment -

          Good point on skipping the commitlog.

          The reason to limit to STC is that I don't think of this as a band-aid for compaction-is-behind (although I suppose it accomplishes that as well) so much as a limit on the worst-case behavior; even when STC is "fully" compacted (i.e. not major compacted but there is nothing left for the bucketing to do) you can have an arbitrary number of sstables contain columns for a given row. Thus, I think using min_compaction_threshold as the cutoff here makes a lot of sense.

          Show
          Jonathan Ellis added a comment - Good point on skipping the commitlog. The reason to limit to STC is that I don't think of this as a band-aid for compaction-is-behind (although I suppose it accomplishes that as well) so much as a limit on the worst-case behavior; even when STC is "fully" compacted (i.e. not major compacted but there is nothing left for the bucketing to do) you can have an arbitrary number of sstables contain columns for a given row. Thus, I think using min_compaction_threshold as the cutoff here makes a lot of sense.
          Hide
          Jonathan Ellis added a comment -

          v2 attached w/ commitlog optimization

          Show
          Jonathan Ellis added a comment - v2 attached w/ commitlog optimization
          Hide
          Sylvain Lebresne added a comment -

          +1 on the technical side

          I'm still far from excited by this because I'm neither convinced that this will be very useful (especially with the max timestamp optimization) nor that it won't be counterproductive in some cases. But for the same reasons I'm not opposing it either if others are more convinced.

          Show
          Sylvain Lebresne added a comment - +1 on the technical side I'm still far from excited by this because I'm neither convinced that this will be very useful (especially with the max timestamp optimization) nor that it won't be counterproductive in some cases. But for the same reasons I'm not opposing it either if others are more convinced.
          Hide
          Jonathan Ellis added a comment -

          It's a pretty simple piece of code, and it's trivially clear that if max timestamp makes it unnecessary, then it's simply a no-op.

          Show
          Jonathan Ellis added a comment - It's a pretty simple piece of code, and it's trivially clear that if max timestamp makes it unnecessary, then it's simply a no-op.
          Hide
          Jonathan Ellis added a comment -

          committed for 1.0.2, that gives us plenty of time to back it out if necessary

          Show
          Jonathan Ellis added a comment - committed for 1.0.2, that gives us plenty of time to back it out if necessary
          Hide
          Jonathan Ellis added a comment -

          reopening for 1.1 since we reverted it out of 1.0.3 in CASSANDRA-3491

          Show
          Jonathan Ellis added a comment - reopening for 1.1 since we reverted it out of 1.0.3 in CASSANDRA-3491
          Hide
          Jonathan Ellis added a comment -

          v3 adds "boolean updateIndexes" to Table.apply; this is safe to turn off for the defragment write, since we're updating w/ exactly the existing data, timestamp and all.

          Also adds a check for cfs.getMinimumCompactionThreshold() > 0.

          Show
          Jonathan Ellis added a comment - v3 adds "boolean updateIndexes" to Table.apply; this is safe to turn off for the defragment write, since we're updating w/ exactly the existing data, timestamp and all. Also adds a check for cfs.getMinimumCompactionThreshold() > 0 .
          Hide
          Sylvain Lebresne added a comment -

          Two small nits:

          • I would prefer using sstablesIterated > cfs.getMinimumCompactionThreshold() rather than >=. I guess I'm afraid that this 'limit worst-case' would get trigger too often. Typically for minThreshold == 2, hoisting as soon as we hit more than 1 sstable feels a bit too much. Again, I have no big argument, it just feels a tad more reasonable with >.
          • We probably should use !CFS.isCompactionDisabled() instead of cfs.getMinimumCompactionThreshold() > 0

          But those minor nits apart, patch lgtm. +1 with or without the changes.

          Show
          Sylvain Lebresne added a comment - Two small nits: I would prefer using sstablesIterated > cfs.getMinimumCompactionThreshold() rather than >= . I guess I'm afraid that this 'limit worst-case' would get trigger too often. Typically for minThreshold == 2, hoisting as soon as we hit more than 1 sstable feels a bit too much. Again, I have no big argument, it just feels a tad more reasonable with >. We probably should use !CFS.isCompactionDisabled() instead of cfs.getMinimumCompactionThreshold() > 0 But those minor nits apart, patch lgtm. +1 with or without the changes.
          Hide
          Jonathan Ellis added a comment -

          committed w/ both changes

          Show
          Jonathan Ellis added a comment - committed w/ both changes
          Hide
          Hudson added a comment -

          Integrated in Cassandra #1219 (See https://builds.apache.org/job/Cassandra/1219/)
          "defragment" rows for name-based queries under STCS, again
          patch by jbellis; reviewed by slebresne for CASSANDRA-2503

          jbellis : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1205403
          Files :

          • /cassandra/trunk/CHANGES.txt
          • /cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java
          • /cassandra/trunk/src/java/org/apache/cassandra/db/Table.java
          Show
          Hudson added a comment - Integrated in Cassandra #1219 (See https://builds.apache.org/job/Cassandra/1219/ ) "defragment" rows for name-based queries under STCS, again patch by jbellis; reviewed by slebresne for CASSANDRA-2503 jbellis : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1205403 Files : /cassandra/trunk/CHANGES.txt /cassandra/trunk/src/java/org/apache/cassandra/db/CollationController.java /cassandra/trunk/src/java/org/apache/cassandra/db/Table.java

            People

            • Assignee:
              Jonathan Ellis
              Reporter:
              Stu Hood
              Reviewer:
              Sylvain Lebresne
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development