[
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
