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

Vinay Chella commented on CASSANDRA-15016:
------------------------------------------

Thank you for the review [~aweisberg]

{quote}Why did you remove this assertion?
{quote}

[assert_almost_equal 
|https://github.com/vinaykumarchella/cassandra-dtest/commit/adcc84d71de6f727d05647b21ff6c48d7bf6920b#diff-0b30b9f097df89d74be1d1af8205ac7eL115]
 is flaky across the versions. I tried to change this to achieve a more 
fine-grained size check. Context: this 
[check|https://github.com/vinaykumarchella/cassandra-dtest/commit/adcc84d71de6f727d05647b21ff6c48d7bf6920b#diff-0b30b9f097df89d74be1d1af8205ac7eL115]
 is to ensure that cleanup and compact did their job, but we end up checking 
the entire node size which might be varying as additional tables that we *may* 
introduce in new releases or already introduced ones from across the versions. 
We also don't put in enough data to make that the dominant cost for test 
keyspaces.

Based on our offline discussion and suggestions, I added table specific data 
size check, and this enables to do data size check for tables that we are 
interested in, with this change now we achieve our goal of size checking 
without depending on external factors of system tables and their size.
{quote}Do you need check bootstrap_from_version? Seems like it is enough to 
just check compatibility_flag_on.
{quote}
I made this better now, instead of exposing it as a {{compatibility_flag_on}}, 
now passing {{bootstrap}} function with compatibility_flag, this makes the 
better use of 
[bootstrap|https://github.com/apache/cassandra-dtest/blob/master/bootstrap_test.py#L46]
 parameter in {{_base_bootstrap_test}}
{quote}Can you better document what is going on and why the compatibility flag 
is being set in the tests, so the next person knows what is going on?
{quote}
I tried to add context in code and also referenced CASSANDRA-13004 in comments.

 
||dtest||upgrade-test||
|[dtest-fix-branch|https://github.com/vinaykumarchella/cassandra-dtest/tree/fix_failing_upgradetest]|[!https://circleci.com/gh/vinaykumarchella/cassandra/tree/3.11.4-tentative.svg?circle-token=237fa63201f848ed64789902be862015f74bec9b!|https://circleci.com/gh/vinaykumarchella/cassandra/352]|



> bootstrap_upgrade_test.py::test_simple_bootstrap_mixed_versions is failing on 
> 3.11.4
> ------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-15016
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15016
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Test/dtest
>            Reporter: Vinay Chella
>            Assignee: Vinay Chella
>            Priority: Minor
>              Labels: dtest, test, upgrade-dtest
>
> {{test_simple_bootstrap_mixed_versions}} is failing due to CASSANDRA-13004, 
> which introduced "cassandra.force_3_0_protocol_version" for schema migrations 
> during upgrades from 3.0.14 upwards. This flag is missing in 
> `test_simple_bootstrap_mixed_versions` while we are adding/bootstrapping 
> 3.11.4 node to an existing 3.5 version of C* node, which resulted in {{ks}} 
> keyspace schema/data not being bootstrapped to the new node.
> I debugged and confirmed that 
> [MigrationManager::is30Compatible|https://github.com/apache/cassandra/blob/cassandra-3.11/src/java/org/apache/cassandra/service/MigrationManager.java#L181-L185]
>  is returning false which is forcing 
> [MigrationManager::shouldPullSchemaFrom|https://github.com/apache/cassandra/blob/cassandra-3.11/src/java/org/apache/cassandra/service/MigrationManager.java#L168-L177]
>  to return false as well.
> *from debug logs:*
> {code:java}
> DEBUG [GossipStage:1] 2019-02-06 23:20:47,392 MigrationManager.java:115 - Not 
> pulling schema because versions match or shouldPullSchemaFrom returned fal
> {code}
> *Failed upgrade tests: 
> [https://circleci.com/gh/aweisberg/cassandra/2593#tests/containers/11]*
> *From failed dtest:* 
> {code:java}
> self = <upgrade_tests.bootstrap_upgrade_test.TestBootstrapUpgrade object at 
> 0x7f1bec192eb8>
>     @pytest.mark.no_vnodes
>     @since('3.10', max_version='3.99')
>     def test_simple_bootstrap_mixed_versions(self):
> >       self._base_bootstrap_test(bootstrap_from_version="3.5")
> upgrade_tests/bootstrap_upgrade_test.py:20: 
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
> _ 
> bootstrap_test.py:114: in _base_bootstrap_test
>     assert_almost_equal(size1, size2, error=0.3)
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
> _ 
> args = (429.36, 133.1), kwargs = {'error': 0.3}, error = 0.3, vmax = 429.36
> vmin = 133.1, error_message = ''
>     def assert_almost_equal(*args, **kwargs):
>         """
>         Assert variable number of arguments all fall within a margin of error.
>         @params *args variable number of numerical arguments to check
>         @params error Optional margin of error. Default 0.16
>         @params error_message Optional error message to print. Default ''
>     
>         Examples:
>         assert_almost_equal(sizes[2], init_size)
>         assert_almost_equal(ttl_session1, ttl_session2[0][0], error=0.005)
>         """
>         error = kwargs['error'] if 'error' in kwargs else 0.16
>         vmax = max(args)
>         vmin = min(args)
>         error_message = '' if 'error_message' not in kwargs else 
> kwargs['error_message']
>         assert vmin > vmax * (1.0 - error) or vmin == vmax, \
> >           "values not within {:.2f}% of the max: {} ({})".format(error * 
> > 100, args, error_message)
> E       AssertionError: values not within 30.00% of the max: (429.36, 133.1) 
> ()
> tools/assertions.py:206: AssertionError
> {code}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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

Reply via email to