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

Ekaterina Dimitrova edited comment on CASSANDRA-15897 at 12/11/20, 4:25 PM:
----------------------------------------------------------------------------

First pass of review done. I don't see any issues with the patch itself, fixed 
formatting at a few places. Latest state of the branch 
[here|https://github.com/ekaterinadimitrova2/cassandra/pull/79]

The failing tests are test issues:
 _TestGossipingPropertyFileSnitch_ - it required 
[update|https://github.com/ekaterinadimitrova2/cassandra-dtest/commit/927a07d0cc9bfcc183d3a0e7c8959cd0c6c88c6a]
 of the VersionedValue in the assertions as the patch introduces a [new 
application state 
update|https://github.com/pcmanus/cassandra/commit/2dd0d1cdbd2c0869bfb8e22be34e0d67a79a8742#diff-9bf2c26bc294ef9085e16bf287490223665eaa2eb8ec24bcf5bd8653c713644bR820].
 _testDropSSTables_ - it turned out the previous commit I added is actually the 
[fix|https://github.com/ekaterinadimitrova2/cassandra/pull/79/commits/dba4a86306bc13f1b421c500570deb3a3f06414f].
 As part of this patch a notification for added/loaded SSTables on start was 
added. This was done in order to accommodate this change and ensure no 
repetitive actions 
[here|https://github.com/pcmanus/cassandra/commit/2dd0d1cdbd2c0869bfb8e22be34e0d67a79a8742#diff-57723368d79933b3d7fcc00ce9d3b98335391901c88cf09176d063fd6eb10ba3R249].

_CompactStorage2to3UpgradeTest_ - I am a bit puzzled about this one. It fails 
by complaining that the nodes do not have this patch. At the same time, I used 
ccm locally to run through the test scenario and everything works smoothly so I 
tend to believe I miss something from the nature of the jvm-upgrade tests

While I am fixing the last mentioned test, I think this patch is ready for 
second review. [~ifesdjeen], [~aleksey], [~marcuse] is anyone of you available? 
Or if [~slebresne] can confirm my corrections/observations?

[CI 
run|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/535/workflows/62835302-6aeb-4a47-b009-c706ced77869]
 [3.0 patch|https://github.com/ekaterinadimitrova2/cassandra/pull/79]
 [DTests 
patch|https://github.com/ekaterinadimitrova2/cassandra-dtest/commit/927a07d0cc9bfcc183d3a0e7c8959cd0c6c88c6a]

 

*NOTE:*  test_prefer_local_reconnect_on_listen_address failed in circleci as I 
forgot to change the dtest repo but the test succeeds locally

 


was (Author: e.dimitrova):
First pass of review done. I don't see any issues with the patch itself, fixed 
formatting at a few places. Latest state of the branch 
[here|https://github.com/ekaterinadimitrova2/cassandra/pull/79]

The failing tests are test issues:
 _TestGossipingPropertyFileSnitch_ - it required 
[update|https://github.com/ekaterinadimitrova2/cassandra-dtest/commit/927a07d0cc9bfcc183d3a0e7c8959cd0c6c88c6a]
 of the VersionedValue in the assertions as the patch introduces a [new 
application state 
update|https://github.com/pcmanus/cassandra/commit/2dd0d1cdbd2c0869bfb8e22be34e0d67a79a8742#diff-9bf2c26bc294ef9085e16bf287490223665eaa2eb8ec24bcf5bd8653c713644bR820].
 _testDropSSTables_ - it turned out the previous commit I added is actually the 
[fix|https://github.com/ekaterinadimitrova2/cassandra/pull/79/commits/dba4a86306bc13f1b421c500570deb3a3f06414f].
 As part of this patch a notification for added/loaded SSTables on start was 
added. This was done in order to accommodate this change and ensure no 
repetitive actions 
[here|https://github.com/pcmanus/cassandra/commit/2dd0d1cdbd2c0869bfb8e22be34e0d67a79a8742#diff-57723368d79933b3d7fcc00ce9d3b98335391901c88cf09176d063fd6eb10ba3R249].

_CompactStorage2to3UpgradeTest_ - I am a bit puzzled about this one. It fails 
by complaining that the nodes do not have this patch. At the same time, I used 
ccm locally to run through the test scenario and everything works smoothly so I 
tend to believe I miss something from the nature of the jvm-upgrade tests

While I am fixing the last mentioned test, I think this patch is ready for 
second review. [~ifesdjeen], [~aleksey], [~marcuse] is anyone of you available? 
Or if [~slebresne] can confirm my corrections/observations?

[CI 
run|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/535/workflows/62835302-6aeb-4a47-b009-c706ced77869]
 [3.0 patch|https://github.com/ekaterinadimitrova2/cassandra/pull/79]
 [DTests 
patch|https://github.com/ekaterinadimitrova2/cassandra-dtest/commit/927a07d0cc9bfcc183d3a0e7c8959cd0c6c88c6a]

> Dropping compact storage with 2.1-sstables on disk make them unreadable
> -----------------------------------------------------------------------
>
>                 Key: CASSANDRA-15897
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15897
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Legacy/Local Write-Read Paths
>            Reporter: Marcus Eriksson
>            Assignee: Ekaterina Dimitrova
>            Priority: Normal
>             Fix For: 3.0.x, 4.0-beta
>
>
> Test reproducing: 
> https://github.com/krummas/cassandra/commits/marcuse/dropcompactstorage



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to