IGNITE-27833 [ducktests] Add Multi-DC Integration Test for Cross-DC Network Partition Resilience - #12740
IGNITE-27833 [ducktests] Add Multi-DC Integration Test for Cross-DC Network Partition Resilience#12740maksaska wants to merge 46 commits into
Conversation
maksaska
changed the title
95f16f1 to
2016503
Compare
anton-vinogradov
left a comment
anton-vinogradov
left a comment
There was a problem hiding this comment.
Reviewed the whole PR. The netem/iptables layering (tcset baseline + per-pair DROP chains, so healing is a pure chain flush) is nicely designed, and the tests read well. A few findings below, most with one-line fixes; the two control_utility.py ones and the connect_timeout default affect shared infrastructure beyond MDC.
Nits not tied to a diff line:
modules/ducktests/tests/tox.inienvliststill containspy38, while the workflow dropped it andtcconfig==0.30.1requires Python >= 3.9 — a localtoxrun will fail on the py38 env. Worth removing it there too.thin_client_test.pyreserves@cluster(num_nodes=8)but allocates 7 (2+2 servers, 1 runner, 2 thin clients).
Replace the always-negated expectAdmissible and tolerateErrors profile options with inadmissible and continueOnError, and reorder the exception handling into a negation-free error-policy ladder (inadmissible -> stopOnError -> continueOnError -> fail). Behavior and precedence are unchanged.
anton-vinogradov
left a comment
anton-vinogradov
left a comment
There was a problem hiding this comment.
Round 2: verified all previous threads against the new head - every one of them is properly fixed (including the tcconfig==0.29.1 route: it requires only Python >=3.7, so the py38 env is consistent again; also compiled with -Pducktests and the strict -Pcheckstyle profile locally - clean).
The final pass still found a few things, two of them merge-blocking:
- blocker: the "codestyle" commit accidentally commented out the Ignite/Zookeeper/Kafka installs in the ducktests Docker image;
- blocker: the new
ClusterState.coordinatorfield breaks positional unpacking ofcluster_state()in existing tests; - one correction to my own round-1 comment about
--user-attributesordering (it is client-sideHashMaporder, notTreeMaporder - details inline, sorry for the detour); - plus a handful of test-strength/robustness notes and two doc nits.
Details inline.
| with cross_dc_network(self.logger, mdc, delay_ms=cross_dc_latency_ms) as net: | ||
| mdc.start_servers() | ||
|
|
||
| # Continuous single-threaded explicit-transaction insert load from the backup DC. |
There was a problem hiding this comment.
This comment contradicts a high-level comment on the test. The high-level comment says that the load runs in the main DC while from what I see in the code the actual runner is started in the backup DC.
| nothing hanging on either half-ring and no suspicious entries in the server logs. | ||
|
|
||
| Data accessibility during the split is deliberately NOT checked: a transactional | ||
| cache needs all partition copies available, which a split-brained half-ring cannot |
There was a problem hiding this comment.
I suspect this statement isn't true: a transactional cache can handle data modification requests if all nodes which backup copies are assigned to are declared FAILED by the discovery component.
So I think we could add a data accessibility check and even a check that data is possible to modify in this scenario as well.
3 participants
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.