cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Yaskevich (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (CASSANDRA-47) SSTable compression
Date Wed, 27 Jul 2011 00:11:09 GMT

     [ https://issues.apache.org/jira/browse/CASSANDRA-47?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Pavel Yaskevich updated CASSANDRA-47:
-------------------------------------

    Attachment: CASSANDRA-47-v4.patch

* I would put the compression option at the CF level. I think it will be particularity useful
at first, when people will want to test compression before trusting it, to be able to switch
a few less important CFs to compression first. On the longer term, compression has probably
a little cost for some workload (especially considering we don't yet have mmaped compressed
file), so it'd be also useful for that. And there is the fact that it's not really more effort
on our side. Nit: I would also call the option more concisely 'compress_sstables'.

  Added option to CF 'compression' + made CLI to recognize it (CLI help is updated also).
Removed that option from cassandra.yaml and related Config and DatabaseDescriptor classes.

* I would move the Metadata class out of CRAR (and call it CompressionMetadata). There is
two reasons:

  Extracted Metadata class from CRAR, named it CompressionMetadata and added Writer to it
+ changed CSW to support that.

* About the Metadata, we should really use a long[] instead of List<Long> for the offsets,
the memory size difference will be significant. Also, since this is a fairly performance sensitive
path, I would create a small Chunk class with a long and an int field to replace the Pair<Long,
Integer> returned by chunckFor, to avoid the boxing/unboxing.

  Done both - changed List<Long> to long[] and Pair<Long, Integer> to Chunk.
 
* Let's add the 'compression algorithm' in the compressionInfo component. It's fine to hard
set it to "Snappy" for writes and ignore the value on read for now.

  Algorithm name is added to the header of CompressionInfo.db in "size + data" format, it
is accessible by "algorithm" variable from CompressionMetadata
 
* In CFS.scrubDataDirectory, it seems that if there is a compressionInfo segment but no data
file, we do not purge orphaned files. I believe this is wrong.
  
  Fixed.

* CRAR.transfer() should honor FileStreamTask.CHUNK_SIZE (as a max at least) for it's buffer.
In particular, having the possibility to allow a 2GB buffer as now (and it's not even unrealistic)
is not a good idea.

  Fixed.

* CompressedSequentialWriter does not honor skipIOCache during flush. I actually think that
flush() is a bit problematic for CSW in that it should only be called when the buffer is full
(or it is the end). It's obviously true of reBuffer and sync too then they use flush. But
flush() and sync() are public. Maybe we could rename flush and sync (in say flushInternal
and syncInternal) and make the public method throw an unsupported exception. Anyway, I think
it would be cleaner if the flush of CSW was calling the one of SW (this would solve the skipIOCache
problem in particular). We can move the buffer write into a protected method in SW and only
override that in CSW, and let the flush of SW untouched otherwise.

  Made a protected flushData method which is now overridden in CSW instead of flush(), didn't
change visibility of the sync and flush methods because this requires further consideration
and should be done in the separate task.
 
* Not totally related to that patch but it seems that in SW, when you call reBuffer, there
is one call to resetBuffer in flush() and one after it. It seems the one in reBuffer is useless
(that would make reBuffer be an alias to flush() really).

  While we have a public flush method resetBuffer() call in there should stay.

* In CSW, why not reuse the buffer where compressed data is used ?

  Fixed.

* I think the truncate methods are not correct for CSW. The offset given is a position in
the uncompressed data, so it will not truncate where it should. For truncateAndClose(), we
should probably not have it public anyway: the only place it is used, it's really close()
that you want to use, and we can do "the right thing" in close for CSW. For truncate, it should
probably throw an unsupported exception.
For ResetAndTruncate, I think we have a harder problem. What the method should do is: seek
back to the chunk containing the wanted destination (imply keeping chunk offsets in memory),
load the chunk in the buffer and position the bufferOffset correctly.

  Made truncateAndClose() protected, changed SSTW to use close (with comment), mark() method
is overridden in CSW and creates a FileMark with chunk offset instead of uncompressed offset.

* In SegmentedFile, I would prefer adding a 'compressed' flag to getBuilder() instead of having
getCompressedBuilder. If only because we should probably add a mmapped compressed mode at
some point.

  That would be changed when and if we decide to use mmaped I/O with compressed files.

* There's an useless added import in CompactionManager. Also a wrongly placed import in SSTW.

  Fixed.

* In CRAR, I would use metada.chunkLength instead buffer.length in most places (decompressChunk
mainly). I know it is the same value right now, but it's really chunkLength that we want here
so it feels more clear (nothing prevents us from using a buffer larger than chunkLength, not
that it would be useful)

  buffer.length always equals to chunkLength, we can't have a buffer of other size because
of decompression needs.

* In SSTR and SSTW, we can use the isCompressed SSTable flag instead of 'if (components.contains(Component.COMPRESSION_INFO))'.

  Removed from SSTW but in the SSTR it is used in the static method where we don't have isCompressed
flag.

> SSTable compression
> -------------------
>
>                 Key: CASSANDRA-47
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-47
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Pavel Yaskevich
>              Labels: compression
>             Fix For: 1.0
>
>         Attachments: CASSANDRA-47-v2.patch, CASSANDRA-47-v3-rebased.patch, CASSANDRA-47-v3.patch,
CASSANDRA-47-v4.patch, CASSANDRA-47.patch, snappy-java-1.0.3-rc4.jar
>
>
> We should be able to do SSTable compression which would trade CPU for I/O (almost always
a good trade).

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message