[
https://issues.apache.org/jira/browse/CASSANDRA-10112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15182468#comment-15182468
]
Stefania edited comment on CASSANDRA-10112 at 3/7/16 2:19 AM:
--------------------------------------------------------------
Thank you for the review.
bq. Can you verify that the failing
{{org.apache.cassandra.io.sstable.SSTableWriterTest.testAbortTxnWithOpenEarlyShouldRemoveSSTable}}
utest is not a regression?
It passes locally and it failed on trunk as well, see [build
751|http://cassci.datastax.com/job/trunk_testall/751/testReport/org.apache.cassandra.io.sstable/SSTableWriterTest/testAbortTxnWithOpenEarlyShouldRemoveSSTable].
bq. It would be nice to use constants instead of magic numbers for
{{StartupException}} exit status codes.
I've introduced 3 generic constants (1: wrong machine state, 3: wrong disk
state, 100: wrong config). I had to change the JNA unavailable exit error from
3 to 1. We could make the constants more specific but we'd have to change more
exit codes.
bq. In {{LogRecord.make()}}, why do we catch {{Throwable}}? Should we be
passing that through {{JVMStabilityInspector}}?
To catch the exceptions thrown by the {{valueOf()}} methods as far as I
remember. I don't see anything else that could throw so I've replaced
{{Throwable}} with {{IllegalArgumentException}}.
bq. {{removeUnfinishedCompactionLeftovers()}} could use some javadocs
(especially explaining the return value).
Added some comments to {{LogTransaction.removeUnfinishedLeftovers()}}.
bq. I have a slight for using the term "directories" instead of "folders" (but
it's not worth changing existing code for this)
You're quite right, folder is a Windows Explorer concept and it is not
necessarily a directory. It didn't take long so I've changed the mentions to
folder that I could find in the {{Log*}} files in {{db.lifecycle}}.
bq. I think this ticket needs a {{doc-impacting}} label
Added.
--
I've restarted one more CI run.
was (Author: stefania):
Thank you for the review.
bq. Can you verify that the failing
{{org.apache.cassandra.io.sstable.SSTableWriterTest.testAbortTxnWithOpenEarlyShouldRemoveSSTable}}
utest is not a regression?
It passes locally and it failed on trunk as well, see [build
751|http://cassci.datastax.com/job/trunk_testall/751/testReport/org.apache.cassandra.io.sstable/SSTableWriterTest/testAbortTxnWithOpenEarlyShouldRemoveSSTable].
bq. It would be nice to use constants instead of magic numbers for
{{StartupException}} exit status codes.
I've introduced 3 generic constants (1: wrong machine state, 3: wrong disk
state, 100: wrong config). I had to change the JNA unavailable exit error from
3 to 1. We could make the constants more specific but we'd have to change more
exit codes.
bq. In {{LogRecord.make()}}, why do we catch {{Throwable}}? Should we be
passing that through {{JVMStabilityInspector}}?
To catch the exceptions thrown by the {{valueOf()}} methods as far as I
remember. I don't see anything else that could throw so I've replaced
{{Throwable}} with {{IllegalArgumentException}}.
bq. {{removeUnfinishedCompactionLeftovers()}} could use some javadocs
(especially explaining the return value).
Added some comments to {{LogTransaction.removeUnfinishedLeftovers()}}.
bq. I have a slight for using the term "directories" instead of "folders" (but
it's not worth changing existing code for this)
You're quite right, folder is a Windows Explorer concept and it is not
necessarily a directory. It didn't take long so I've changed the mentions to
folder that I could find in the {{Log*}} files in {{db.lifecycle}}.
bq. I think this ticket needs a {doc-impacting}} label
Added.
I've restarted one more CI run.
> Refuse to start and print txn log information in case of disk corruption
> ------------------------------------------------------------------------
>
> Key: CASSANDRA-10112
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10112
> Project: Cassandra
> Issue Type: Improvement
> Components: Local Write-Read Paths
> Reporter: Stefania
> Assignee: Stefania
> Labels: doc-impacting
> Fix For: 3.x
>
>
> Transaction logs were introduced by CASSANDRA-7066 and are read during
> start-up. In case of file system errors, such as disk corruption, we
> currently log a panic error and leave the sstable files and transaction logs
> as they are; this is to avoid rolling back a transaction (i.e. deleting
> files) by mistake.
> We should instead look at the {{disk_failure_policy}} and refuse to start
> unless the failure policy is {{ignore}}.
> We should also consider stashing files that cannot be read during startup,
> either transaction logs or sstables, by moving them to a dedicated
> sub-folder.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
