[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15324708#comment-15324708
]
Kaide Mu commented on CASSANDRA-11875:
--
bq. BigVersion.supportsWritingversion(version)
It is now implemented, now we can check if a version different from Version is
supported for writing, I use BigFormat.latestVersion.supportesWritingversion in
StandaloneConverter to check if a given version is supported.
bq. The idea is to abstract only the identical part, leaving specific parsing
to each class (so you can probably extract the parseArgs code from inside
Options).
By doing so I think we have to create a external Option or ConverterOption
abstract class andextend from them in StandaloneUpgrader or
StandaloneConverter. Another way is making StandaloneConverter.Options public.
[~pauloricardomg] do you think is the right way?
bq. The testUnsupportedVersionShouldFail is failing, you should generally use
this format to assert that exceptions are thrown while making the test pass
This is also done, I'll submit a patch once previous issue is solved, but I'm
not sure if there is any other RuntimeException is thrown, do you think we
should create a UnsupportedWritingExeption to ensure it?
Thanks!
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15316642#comment-15316642
]
Paulo Motta commented on CASSANDRA-11875:
-
Thanks for the update! See follow-up comments below:
bq. Changed isCompatibleForWriting(String version) -> isCompatibleForWriting()
since we are dealing directly with versions so it would be redundant to pass it.
Good idea, that's probably better.
bq.
DatabaseDescriptor.getSSTableFormat().info.getVersion(options.version).isCompatibleForWriting(),
or maybe there's some way to obtain it?
Maybe you can add a {{BigVersion.supportsWritingversion(version)}} that could
wrap that.
bq. I modified Options access level to protected and parseArgs now varies on
boolean isLatestVersion, so now StandaloneUpgrader only contains the execution
code which is identical than StandaloneConverter, do you think we should
abstract it into a common method SSTableConverter.execute(Option options)?
The idea is to abstract only the identical part, leaving specific parsing to
each class (so you can probably extract the {{parseArgs}} code from inside
{{Options}}). The main class of StandaloneUpgrader would look more or less like:
{code:java}
public static void main(String args[])
{
Options opts = parseArgs(args);
StandaloneConverter converter = new StandaloneConverter(opts)
converter.convert()
}
{code}
With this approach {{StandaloneUpgrader}} doesn't need to be a subclass of
{{StandaloneConverter}}.
On tests, it seems you're using {{assertNotSame}} to check objects are not
equals, but {{assertNotSame}} only compare object equality ({{obj1 != obj2}}),
not if objects are effectively not equal ({{!obj1.equals(obj2)}}). You should
probably use {{assertFalse(object.equals(other))}} to assert non-equality;
The {{testUnsupportedVersionShouldFail}} is failing, you should generally use
this format to assert that exceptions are thrown while making the test pass:
{code:java}
try {
methodThatShouldThrow();
fail( "My method didn't throw when I expected it to" );
} catch (MyException expectedException) {
}
{code}
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15313350#comment-15313350
]
Kaide Mu commented on CASSANDRA-11875:
--
Hello Paulo, new
[patch|https://github.com/apache/cassandra/compare/trunk...kdmu:trunk-11875?expand=1]
available. Changed isCompatibleForWriting(String version) ->
isCompatibleForWriting() since we are dealing directly with versions so it
would be redundant to pass it.
bq.We should probably add a Descriptor.isCompatibleForWriting so we can check
that directly from the descriptor, and use that to throw an exception on
SSTableConverter if the version is incompatible
Added Descriptor.isCompatibleWriting(), but in case of {{SSTableConverter}} we
don't have specified version {{Descriptor}} so we have to check it from
{{Version}}
{{DatabaseDescriptor.getSSTableFormat().info.getVersion(options.version).isCompatibleForWriting()}},
or maybe there's some way to obtain it?
bq.The idea of your WIP is correct, but instead of calling the main method of
StandaloneUpgrader you could create a common method that receives an Options
object, which in the case of the upgrader it would have a special flag
indicating upgrade to latest version. The argument parsing and Options creation
could be removed from the Options object to the main class so it's different
between the converter and the upgrader. You can do different printing based on
the options flag.
I modified Options access level to protected and parseArgs now varies on
boolean isLatestVersion, so now {{StandaloneUpgrader}} only contains the
execution code which is identical than {{StandaloneConverter}}, do you think we
should abstract it into a common method SSTableConverter.execute(Option
options)?
Thanks.
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15312874#comment-15312874
]
Paulo Motta commented on CASSANDRA-11875:
-
Thanks for the update. Overall the patch is looking much better, just a few
minor nits:
* CONVERT_SSTABLES("Converte sstables") -> CONVERT_SSTABLES("Convert sstables"),
* {{compatibleForWriting}} set on BigFormat is a constant, so should probably
be named {{supported_write_formats}} and move next to {{current_version}} and
{{earliest_supported_version}} to be consistent with other constants in
{{BigVersion}}
* We should probably add a {{Descriptor.isCompatibleForWriting}} so we can
check that directly from the descriptor, and use that to throw an exception on
{{SSTableConverter}} if the version is incompatible
** You may have a look on a similar situation on
{{ColumnFamilyStore.loadNewSSTables}}
On the tests, it would be nice if you could read the data before and after
converting (SELECT) to verify it's consistent. Similarly, you should check that
commit log lower bound is different from {{ReplayPosition.NONE}} before the
conversion. On {{testUnsupportedVersionShouldFail}} the test does not guarantee
the program failed with an error, so it's probably better to test that an
exception is being thrown when you try to convert with {{SSTableConverter}}
with an unsupported version.
bq. I'm not sure if we have to make StandaloneUpgrader subclass of
StandaloneConverter. I did a WIP for that, since we have to validate passed
args (validate and add latestVersion to it), we should maintain original
parseArgs of StandaloneUpgrader, also since printUsage is different from
StandaloneConverter we should also maintain it.
The idea of your WIP is correct, but instead of calling the main method of
{{StandaloneUpgrader}} you could create a common method that receives an
{{Options}} object, which in the case of the upgrader it would have a special
flag indicating upgrade to latest version. The argument parsing and Options
creation could be removed from the Options object to the main class so it's
different between the converter and the upgrader. You can do different printing
based on the options flag.
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15312634#comment-15312634
]
Kaide Mu commented on CASSANDRA-11875:
--
New [patch|https://github.com/kdmu/cassandra/tree/trunk-11875] available.
Working test suit included.
I'm not sure if we have to make {{StandaloneUpgrader}} subclass of
{{StandaloneConverter}}. I did a
[WIP|https://gist.github.com/anonymous/29b850c4abed44702c7490606c09fbdc] for
that, since we have to validate passed args (validate and add latestVersion to
it), we should maintain original parseArgs of {{StandaloneUpgrader}}, also
since printUsage is different from {{StandaloneConverter}} we should also
maintain it.
Looking forward for your feedback,
Thanks!
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15306004#comment-15306004
]
Paulo Motta commented on CASSANDRA-11875:
-
Thanks for the update! See follow-up comments below:
* There is still some common code between {{StandaloneConverter}} and
{{StandaloneUpgrader}} main methods, most of it can be extracted to common
methods use by both.
* The supported version check will probably be used in other places, so we
should probably move it to {{BigVersion}}. There are already {{isCompatible}}
and {{isCompatibleForStream}} methods, so we can maybe add a
{{isCompatibeForWriting}}. It would be nice if you could add a unit test that
checks that trying to converting to an unsupported version should fail.
bq. Also added a new Test suit SSTableConversionTest, but it has some problem
when performing conversion.
It seems this is due to re-loading the schema on
{{Schema.instance.loadFromDisk(false)}} on {{StandaloneConverter}}. Since we
want to focus testing the conversion itself, we will probably have more
flexibility testing the internal class {{SSTableConverter}}, so we don't have
to add special options for testing on {{StandaloneConverter}} and can also play
around with SSTableReaders directly.
Instead of basing our tests on {{SSTableRewriterTest}} as initially discussed,
it's probably more convenient to base it on {{CQLTester}} since we will be
doing data conversions and testing this at a higher level with CQL is the only
way to ensure converted data is being interpreted correctly. The tests should
have more or less the following structure:
* Insert Data with CQL
* Flush to disk
* Read and verify data in current version with CQL
* Keep reference to original SSTableReaders and cleanup ColumnFamilyStore
(clearUnsafe)
* Perform conversion on original SSTableReaders
* Verify metadata was converted correctly on converted SSTableReaders
* Add converted SSTableReaders to ColumnFamilyStore (addSSTable)
* Read and verify data in converted version with CQL
For the mb to ma conversion, since there is no data conversion involved (only
metadata), you can use {{SimpleQueryTest.testTableWithoutClustering}} as a
example to write the first test case. You may use utility {{CQLTester}} utility
methods such as {{flush()}} and {{getCurrentColumnFamilyStore()}} to flush and
access {{ColumnFamilyStore}}.
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11875) Create sstableconvert tool with support to ma format
[
https://issues.apache.org/jira/browse/CASSANDRA-11875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15305295#comment-15305295
]
Kaide Mu commented on CASSANDRA-11875:
--
Just unified things into SSTableConverter and StandaloneConverter.
Also added a new Test suit SSTableConversionTest, but it has some problem when
performing conversion.
{{ERROR 10:39:13 Attempting to load already loaded table
SSTableRewriterTest.Standard1}}
[Patch|https://github.com/kdmu/cassandra/commit/e8637a531e874310a49bc5dff71c94b8841c2e09]
(pushed to trunk instead of a temporary working branch, will fix it)
> Create sstableconvert tool with support to ma format
>
>
> Key: CASSANDRA-11875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11875
> Project: Cassandra
> Issue Type: Sub-task
> Components: Tools
>Reporter: Paulo Motta
>Assignee: Kaide Mu
>Priority: Minor
> Attachments: trunk-11875-WIP-V1.patch
>
>
> Currently {{Upgrader}} receives an sstable in any readable format, and writes
> into {{BigFormat.getLatestVersion()}}. We should generalize it by making it
> receive a {{target}} version and probably also rename it to
> {{SSTableConverter}}.
> Based on this we can create an {{StandaloneDowngrader}} tool which will
> perform downgrade of specified sstables to a target version. To start with,
> we should support only downgrading to {{ma}} format (from current format
> {{mb}}), downgrade to any other version should be forbidden. Since we already
> support serializing to "ma" we will not need to do any data conversion.
> We should also create a test suite that creates an sstable with data in the
> current format, perform the downgrade, and verify data in the new format is
> correct. This will be the base tests suite for more advanced conversions in
> the future.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
