[
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 1/10/21, 11:06 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-
Tests
[fixed|https://github.com/ekaterinadimitrova2/cassandra/pull/79/commits/307ba7f6cf00364280b4a8155444f96689c9d5c2],
a bit of time is needed after the upgrade for the nodes to settle down
-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-
Tests
[fixed|https://github.com/ekaterinadimitrova2/cassandra/pull/79/commits/307ba7f6cf00364280b4a8155444f96689c9d5c2],
a bit of time is needed after the upgrade for the nodes to settle down
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
> 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: [email protected]
For additional commands, e-mail: [email protected]
