[
https://issues.apache.org/jira/browse/CASSANDRA-9669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14745367#comment-14745367
]
Branimir Lambov commented on CASSANDRA-9669:
--------------------------------------------
[Memtable.isCleanAfter|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-f0a15c3588b56c5ce53ece7c48e325b5R171]
doesn't look right. This guarantees that it does not contain data _before_ the
given position, and thus {{CFS.forceFlush(ReplayPosition)}} does not appear to
use it correctly.
[Descriptor.Version.isCompatible|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-d1b9592895e36631679f1d5cab0f61f1R97]
is suspect in the context of this ticket, "ka" reader says compatible with
"kb" data but read will fail. Later readers (e.g. "la") will also fail to read
these tables but say they are compatible. Could this cause trouble?
The new format needs to be added to {{LegacySSTableTest}} and
{{test/data/legacy-sstables}} and included in 2.2 and 3.0 versions of the patch.
\\
\\
Nits:
[Memtable.approximateCommitLogLowerBound|https://github.com/belliottsmith/cassandra/blob/d190c29d5e24e0b2a93a844d250695107e8bb942/src/java/org/apache/cassandra/db/Memtable.java#L64]:
This must satisfy {{approximateCommitLogLowerBound <= commitLogLowerBound}}
(after discarding predecessor), mustn't it? Could you add a comment to say so?
[Memtable.forceFlush(ReplayPosition)|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-98f5acb96aa6d684781936c141132e2aR972]
no longer loops through index CFSes, can you add a comment to state why this
is the right thing to do?
[writeSortedContents|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-f0a15c3588b56c5ce53ece7c48e325b5R379]:
duplicate log?
[Descriptor.Version version
text|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-d1b9592895e36631679f1d5cab0f61f1R56]:
2._1_.7; j versions are compatible according to {{isCompatible}}, shouldn't
comment be retained?
[LegacyMetadataSerializer.serialize|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-0f1c5b6135b34548f033e5168f6761a5R96]:
No need to declare {{commitLogUpperBound}} here. Declare with assignment below
to ease reading.
[SSTableMetadataViewer|https://github.com/apache/cassandra/compare/trunk...belliottsmith:9669-2.1#diff-63785fd0e03996f22c0c2c38eb137f7fR71]:
Could use some text describing what's being printed.
> If sstable flushes complete out of order, on restart we can fail to replay
> necessary commit log records
> -------------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-9669
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9669
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Reporter: Benedict
> Assignee: Benedict
> Priority: Critical
> Labels: correctness
> Fix For: 3.x, 2.1.x, 2.2.x, 3.0.x
>
>
> While {{postFlushExecutor}} ensures it never expires CL entries out-of-order,
> on restart we simply take the maximum replay position of any sstable on disk,
> and ignore anything prior.
> It is quite possible for there to be two flushes triggered for a given table,
> and for the second to finish first by virtue of containing a much smaller
> quantity of live data (or perhaps the disk is just under less pressure). If
> we crash before the first sstable has been written, then on restart the data
> it would have represented will disappear, since we will not replay the CL
> records.
> This looks to be a bug present since time immemorial, and also seems pretty
> serious.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
