[ 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:26 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 with the proposed dtest patch 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] *NOTE:* test_prefer_local_reconnect_on_listen_address failed in circleci as I forgot to change the dtest repo but the test succeeds locally > 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