[ 
https://issues.apache.org/jira/browse/CASSANDRA-10109?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14728051#comment-14728051
 ] 

Benedict commented on CASSANDRA-10109:
--------------------------------------

So, I have pushed a version 
[here|github.com/belliottsmith/cassandra/tree/10109-3.0] with some largeish 
changes. These are all *suggestions*, and I would like to emphasize that you 
should challenge anything you think is a retrograde step for clarity. I just 
felt the code was likely to be tricky for anybody besides the two of us to 
maintain as it stood, and I hope you don't mind my having a go at (hopefully) 
making it more approachable to a non-author. To summarise, I have:

* Combined both atomic/non-atomic {{DirectoryStream}} paths
* Split out all of the inner classes into their own files
* Merged the {{TransactionFile}} and {{TransactionData}} classes into 
{{LogFile}}, and moved the resource management out into the {{TransactionLog}} 
(which is all that needs it)
* Merged the checksum portion of the log file lines with the {{Record}} 
functionality, so all parsing is in one place
* Moved all of the validation logic into one place, including checksum 
verification
* Made validation throw no exceptions, only log and return the result, for 
consideration of the caller
* Made extensive use of streams where possible and clarity was helped, since 
these are all low traffic codepaths

I haven't quite finished looking it all over and thinking about the behaviour. 
It's worth noting that four of your unit tests now fail, however looking at 
them it appears that the correct behaviour is to fail - although perhaps not if 
we are just listing. That requires some consideration.

Let me know what you think.

> Windows dtest 3.0: ttl_test.py failures
> ---------------------------------------
>
>                 Key: CASSANDRA-10109
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10109
>             Project: Cassandra
>          Issue Type: Sub-task
>            Reporter: Joshua McKenzie
>            Assignee: Stefania
>              Labels: Windows
>             Fix For: 3.0.0 rc1
>
>
> ttl_test.py:TestTTL.update_column_ttl_with_default_ttl_test2
> ttl_test.py:TestTTL.update_multiple_columns_ttl_test
> ttl_test.py:TestTTL.update_single_column_ttl_test
> Errors locally are different than CI from yesterday. Yesterday on CI we have 
> timeouts and general node hangs. Today on all 3 tests when run locally I see:
> {noformat}
> Traceback (most recent call last):
>   File "c:\src\cassandra-dtest\dtest.py", line 532, in tearDown
>     raise AssertionError('Unexpected error in %s node log: %s' % (node.name, 
> errors))
> AssertionError: Unexpected error in node1 node log: ['ERROR [main] 2015-08-17 
> 16:53:43,120 NoSpamLogger.java:97 - This platform does not support atomic 
> directory streams (SecureDirectoryStream); race conditions when loading 
> sstable files could occurr']
> {noformat}
> This traces back to the commit for CASSANDRA-7066 today by [~Stefania] and 
> [~benedict].  Stefania - care to take this ticket and also look further into 
> whether or not we're going to have issues with 7066 on Windows? That error 
> message certainly *sounds* like it's not a good thing.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to