[
https://issues.apache.org/jira/browse/CASSANDRA-10421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14952596#comment-14952596
]
Stefania commented on CASSANDRA-10421:
--------------------------------------
bq. I am not sure if losing atomicity on commit is safe. I think it isn't
because you could have a log asking you to remove some files that are
ancestors, but no log records telling you to add their replacements in a
different location. Do the replacements just silently get added in that case?
If so then yes I think it's fine.
We strictly do what the txn log files say, if the user deleted the txn log file
in the folder where the replacements are, then we would not add them. This is
probably a problem, see more at the end of this comment.
bq. When writing a commit if we fail to write the commit to all files does that
mean we don't attempt to transition to the new state?
Yes the transaction would be rolled back. {{LogTransaction}} is only used by
{{LifecycleTransaction}} so you need to look at its code. It calls
{{log.commit()}} as the first entry in its {{doCommit()}} and it throws in case
of error; so if we cannot write to any txn logs we fail the commit and the
transaction is rolled back.
bq. The transition to the new state is always going to be applying deletions to
obsolete sstables right?
Yes, on successful commit the obsoleted sstables are scheduled to be deleted as
soon as all the references are released.
bq. What about failing to add a record?
Failing to add a NEW record would result in an exception when creating the
sstable writer. Failing to add an OLD record would result in an exception
thrown or returned by {{LifecycleTransaction}} depending on whether it is
called in {{doPrepare}} or {{doAbort}} (or post cleanup and cancel).
bq. If that fails does it abort the transaction?
{{doPrepare}} can throw but {{doAbort}} cannot. This second case is for non
original tables however, i.e. if they were opened early.
bq. Would that block compaction progress?
Yes a compaction in progress is a transaction, if {{prepare}} throws then the
transaction is aborted. See {{CompactionAwareWriter.finish}}
--
bq. This is common to both branches of shouldCommit. Also shouldn't both wait
for deletions even if none are expected since you are checking to see if it is
done incorrectly?
Fixed, thank you.
bq. In my mind the only open question is how to handle the scenario where the
log on one disk has you remove files while the log on another disk doesn't have
you add their replacements because it's corrupt. That wouldn't happen if the
log was replicated to all the disks. Even if it was though, and you lost a
disk, maybe you need to know you lost a disk containing replacement files and
that the transaction actually needs to handle that case where it can't get to
the aborted or committed state.
I agree this is the sticky point. Assuming that we should not remove the
ancestors (which seems to be the case from your earlier comments on Jonathan's
preference) then I agree: we not only need to duplicate the logs, we also need
to detect corrupt replacements. At the moment we focus on detecting corrupt
ancestors, not replacements. It is trickier to check if replacements are valid
because the files do not yet exist when we start tracking replacements.
I think we should try to detect if some log files or replacements are missing
and in this case log an error and leave everything as is. We can add more
information to the final record to detect this cases. WDYT?
> Potential issue with LogTransaction as it only checks in a single directory
> for files
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-10421
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10421
> Project: Cassandra
> Issue Type: Bug
> Reporter: Marcus Eriksson
> Assignee: Stefania
> Priority: Blocker
> Fix For: 3.0.0 rc2
>
>
> When creating a new LogTransaction we try to create the new logfile in the
> same directory as the one we are writing to, but as we use
> {{[directories.getDirectoryForNewSSTables()|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/lifecycle/LogTransaction.java#L125]}}
> this might end up in "any" of the configured data directories. If it does,
> we will not be able to clean up leftovers as we check for files in the same
> directory as the logfile was created:
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/lifecycle/LogRecord.java#L163
> cc [~Stefania]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
