Repository navigation
Bug#121063 Members of one group assign different GNOs to the same transaction - #736
matias-sanchez wants to merge 4 commits into
Conversation
jujose-1
left a comment
There was a problem hiding this comment.
Hi Matias,
Thank you for the contribution and for working on this fix. I’ve left a few review comments for your consideration. Please take a look when you have a chance.
Regards,
Justin Jose
288df77 to
267fed8
Compare
|
Hi @jujose-1, thanks for taking the time to review this and for the comments. Added the requested changes. |
jujose-1
left a comment
There was a problem hiding this comment.
Hi @matias-sanchez ,
Thank you for updating the patch and addressing the earlier feedback. The production change looks good to me. I’ve left one additional comment regarding the unit-test setup.
I’ll also wait for @tiagoportelajorge 's review and any additional feedback he may have.
Thank you again for working on this.
Regards,
Justin Jose
|
My proposal is to add two debug asserts:
|
|
Thanks @kamil-holubicki . However, I'm not sure on how to safely add these asserts, because the issue covered is on multi primary and I'm not sure how to add that assert so that it does not fire when |
|
Thanks, @matias-sanchez , for updating the unit test. The revised test now uses valid memberships and verifies the same reservation across the view change. This looks good to me. Thanks also, @kamil-holubicki, for the suggestions.
My understanding is that the proposed assertions would cover the scenario reported in this bug, but they may not hold for remotely allocated synodes, where the synode can legitimately carry the allocating leader’s node index rather than the proposer’s. Given that the reported issue involves a locally allocated reservation, and the current fix scopes the ownership validation to that path, I suggest keeping this PR focused on that change. The acceptor side does not currently have the allocation provenance needed to apply the same invariant safely, so I would prefer not to add the proposed assertions here. |
…t GNOs to the same transaction https://perconadev.atlassian.net/browse/PS-11530 Upstream bug: Bug#121063: https://bugs.mysql.com/bug.php?id=121063 Problem: Two members of the same group assign different GNOs to the same transaction, in multi-primary mode, when a member leaves and rejoins during a rolling restart under write load. The group splits into internally consistent sets that disagree on the GTID of the same transaction. It can surface on the `group_replication_applier` channel as Error_code 1032 or 1062, or stay silent with every member ONLINE and the disagreement present only in the binary logs. Cause: A reserved synode is only ours while we still hold the node index it was reserved under. `local_synode_allocator` stamps the member's current index into the synode, `synode.node = my_nodeno`. Still inside `reserve_synode_number`, the task yields in the `while (too_far(*msgno))` loop at `TIMED_TASK_WAIT`. During that yield, `site_install_action` reassigns `site->nodeno`. The reservation still carries the old index, so `proposer_task` brands and proposes into a slot that now belongs to another node. The header comment of `xcom_base.cc` states the rule: only node N may propose a value for synode {X N}. With two proposers on the same slot at `cnt=0`, `acceptor.promise` is never raised, since it is assigned in exactly one place, inside `handle_simple_prepare`, which is the phase 1 decision. Both proposals are accepted, both are learned, and `handle_learn` keeps whichever LEARN arrived first because of the `/* Avoid re-learn */` guard. Members that heard different values first deliver different payloads. Solution: Before proposing, verify the reservation still carries the member's own node index. If it does not, drop it through `retry_new` and take a new one. This is the same check `incr_msgno` already makes whenever it advances, with the comment `In case site and node number has changed`. The client transaction is not lost, it goes out in a slot that does belong to the member. Ported from upstream MySQL PR mysql/mysql-server#736 by Matias Sanchez. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| reserved under. A view change that renumbers us hands that slot to another | ||
| node, which may reserve it as well. */ | ||
| bool_t reservation_is_stale(synode_no msgno) { | ||
| site_def const *site = find_site_def(msgno); |
There was a problem hiding this comment.
I need to think a bit more about this, because I don't know exactly how find_site_def behaves for synodes that were still in the proposal phase. It works well for synodes that are already in the stream and accepted by the group.
That synode might still happen, but now necessarily with the data you are proposing. That is why we have checks like if (match_my_msg(ep->p->learner.msg, ep->client_msg->p))
There was a problem hiding this comment.
Hi Tiago. As far as I can tell find_site_def does not treat the two cases differently: it walks the site_defs list comparing the synode against each site's start, and has no notion of whether the synode was proposed, accepted or executed.
On match_my_msg, I think it catches a different scenario: it runs after finished(p), so the value is already learned and kept by handle_learn's re-learn guard by then.
Let me know if you think I might be missing something. It is tough source code i'm not entirely familiar with.
|
HI @matias-sanchez , thank you for working on this! I've reopened a conversation because I think that the code can be simplified. I've also dropped some food for thought, even for me to double-check the behaviour of |
|
Thanks @jujose-1 and @tiagoportelajorge for the review. @tiagoportelajorge I will look deeper into the code on the points you raised and update soon, as at first glance I am still not sure how it could be simplified, so let me know any further thoughts or guidance you may have. |
Hi @matias-sanchez, Please check my comment in the reopened thread:
Regards, Tiago |
|
Hi @tiagoportelajorge , thanks for reposting your comment as I had missed that one. I tested you're proposal and you were right, that check was not needed, the one inside the loop is enough :) . Just pushed a commit removing it and leaving only the one inside the loop. |
|
Hi @tiagoportelajorge @jujose-1, only posting back to check with you if something is still pending at my side on this PR, or are we good? |
|
Hi @matias-sanchez, Thank you for checking in. There is nothing pending from your side at the moment. We are having an internal discussion on the process for integrating the change into trunk. We’ll let you know if anything further is needed from your side. Thank you again for the contribution. Regards, |
|
Hi @jujose-1 thanks u so much for the reply! cheers |
|
/codex |
|
✅ Codex PR Review completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Reviewed the four changed files at 549d04d1. Found one Windows maintainer-build issue in the new test's header order (inline). No additional high-confidence defects found in the reservation guard or retry path. Static review only; no PR code or tests were executed, as required by this workflow. Prior inline review threads were unavailable under the read-access policy.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
ab.chatgpt.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
Generated by Codex PR Review for #736 · codex · gpt60 · 175.9 AIC · ⌖ 24.1 AIC · ⊞ 14K
Comment /codex to run again
| #include <xcom/xcom_profile.h> | ||
| #include <xcom_vp.h> | ||
| #include "gcs_base_test.h" |
There was a problem hiding this comment.
[P2] Handle the Windows header-order warning in the new test
Including xcom_vp.h before gcs_base_test.h brings in the Windows headers through the bundled SunRPC headers before the standard integer headers, triggering the known UINT8_MAX macro redefinition (MSVC C4005). The existing gcs_xcom_site_def-t.cc uses this same include order and has a file-specific /wd4005 in unittest/gunit/libmysqlgcs/CMakeLists.txt; that suppression does not cover this new test, and mysqlgcs's suppression is PRIVATE. With MYSQL_MAINTAINER_MODE enabling /WX, the new test therefore breaks the Windows build. Include gcs_base_test.h first, as the other XCom tests do, or extend the existing Windows suppression to this source.
|
Hi, thank you for your contribution. Your code has been assigned to an internal queue. Please follow |
Blueprint https://blueprints.launchpad.net/percona-server/+spec/audit-log-default-db Add field `DB' for `MYSQL_AUDIT_GENERAL_STATUS' records. ========================================== [mysql#786] Bug 1610242: Reading past the end of heap buffer on saving the default DB in audit p Original title: [mysql#786] Bug 1610242: Reading past the end of heap buffer on saving the default DB in audit plugin memcpy in the call memcpy(local->db, event_general->general_query, sizeof(local->db)); always copies `NAME_LEN' bytes, but the real length of the `general_query' can be less than `NAME_LEN' Fix is to copy `general_query_length' bytes. ========================================== [#930] Bug 1613647: Sporadic audit_log_filter_users failures Insert silent statements (which aren't are grepped away) before connects to make sure the previous statement we which want to see in the audit log actually gets there before the connect. ========================================== [#941] Bug 1617833: audit_log_default_db failures 1. Insert silent statement after UNINSTALL PLUGIN to make sure that SHOW WARNINGS is logged before the statement from the next connection. 2. Insert silent statement before establishing the connection which does 'INSTALL PLUGIN', make sure 'SELECT * FROM t' gets to the log. 3. Make reconnect in case of error optional for 'change_user' mysqltest command ========================================== [#1682] Bug 1650294: Test main.audit_log_filter_users is unstable The source of instability is the fact that event of creating new connection and closing previous one are not synchronized. It makes "Quit" record to appear after "Connect" one. Fix is to replace `include/wait_until_disconnected.inc' which waits for client connection to be closed with `include/wait_until_count_sessions.inc' which waits for specific `Threads_connected' value which is updated after `MYSQL_AUDIT_NOTIFY_CONNECTION_DISCONNECT'. ========================================== [#1762] Bug 1626559: Test `main.audit_log_default_db' is unstable Sometimes "SHOW WARNINGS" appears in the audit log around UNINSTALL PLUGIN / INSTALL PLUGIN. This is the warning which MySQL issue about plugin being in use. Normally warning should not make it to audit log because test case truncates log after UNINSTALL PLUGIN. But sometimes mysql-test picks this warning up after plugin has been reinstalled and it gets logged into audit log. This patch makes sure that test execution continues only when plugin disappeared from the `mysql.plgins', which happens after `deinit' is called and all plugin is fully shut down. ========================================== [mysql#736] Allow to log only queries for specific DBs in audit log plugin Blueprint: [https://blueprints.launchpad.net/percona-server/+spec/audit-filer-db] Add two global variables: - `audit_log_include_databases' comma separated list of databases to include in audit logging - `audit_log_exclude_databases' comma separated list of databases to exclude from audit logging Allow escaped backticks in the `next_word' function. This change needs to be backported to the 5.6. Patch changes behaviour of user and database filters. User filter becomes case sensitive for user name and case insensitive for host name. Database filter becomes case insensitive. [#931] Bug 1617828: audit_log_filter_db failures The fix increases timeout for event shot wait to 10 minutes and adds silent query at the end of the event. ============================================================ [mysql#826] Add filtering by `sql_command' Blueprint: https://blueprints.launchpad.net/percona-server/+spec/audit-filer-command - add option `audit_log_include_command' to specify commands to log - add option `audit_log_exclude_command' to specify commands to exclude from logging - backport changes for `next_word' fixing the capture of default database with quoted backticks. - backport case-sensitivity fix for account filters ========================================== [#1381] Bug 1650321: Valgrind error on main.audit_log_filter_commands Valgrind complained about `cmd' being used uninitialized. However, I don't a path when it could happen, since the first command was to initialize it unconditionally. Still this error message pointed me to the fact that we don't need `cmd' at all, we can simply use `name' and `length' provided as arguments. ========================================== [#1444] Bug 1663251: ASan errors on main.audit_log_filter_commands `my_hash_search' takes two arguments - key to search for and it's length. When length is 0, `my_hash_search' considers that key has the same length as hash length. Since this behaviour may be relied on somewhere, the fix is to avoid calling `my_hash_search' for empty strings. ========================================== [#846] Bug 1614444: Assertion `local->stack.frames[local->stack.top].query == event_general->general_query.str' failed It is a bug which can affects filtering by DB for triggers. There is a trigger CREATE TRIGGER tr1 BEFORE INSERT ON t1 FOR EACH ROW SET @Aux=1; and query INSERT INTO t1(c5,c6)VALUES (1,0); Lets see which events are sent to the audit_log: 1. TABLE ACCESS event for "INSERT INTO t1(c5,c6)VALUES (1,0)" 2. STATUS event for "SET @Aux=1" 3. STATUS event for "INSERT INTO t1(c5,c6)VALUES (1,0)" When (1) comes, plugin pushes "INSERT INTO t1(c5,c6)VALUES (1,0)" to the top of the stack, so it can match it later with STATUS event. When (2) comes, plugin is trying to match "SET @Aux=1" against the stack top. But "SET @Aux=1" isn't on the stack since it doesn't access any database. In debug mode we see the assertion failure. In the release mode, "SET @Aux=1" will be attributed as accessing table "t1", which is wrong. The fix is to replace assert with if statement. Now the query the within trigger which doesn't access any database will always be logged just like any similar query executed outside of the trigger. ============================================================ [#1176] Bug 1641910: Trying to set audit_log_exclude_accounts crashes server The root cause of this bug is that logic around filtering variables rely heavily on the fact that CHECK and UPDATE functions are always called. This is not true when variables passed via defaults file or set as command line options. The fix is add special handling for filtering variables on startup. ========================================== [#1451] Bug 1666496: Server crashes on startup if plugin is unable to create file pointed by --audit_log_file In order to expose this bug we need to set both --audit-log-exclude-commands and --audit-log-file at start up. Second one has to be incorrect, so that plugin would refuse to start with error: mysqld: File './data-dir/mysql-audit.json' not found (Errcode: 2 - No such file or directory) 2017-02-21T11:58:42.392280Z 0 [ERROR] Plugin audit_log reported: 'Cannot open file ./data-dir/mysql-audit.json.' 2017-02-21T11:58:42.392350Z 0 [ERROR] Plugin audit_log reported: 'Error: No such file or directory' 2017-02-21T11:58:42.392355Z 0 [ERROR] Plugin 'audit_log' init function returned error. 2017-02-21T11:58:42.392360Z 0 [ERROR] Plugin 'audit_log' registration as a AUDIT failed. The cause for the crash is common with bug 1641910. MEMALLOC'ed variables are not properly initialized by server (UPDATE and CHECK functions aren't invoked). The fix is to make sure that initialization of MEMALLOC'ed variables is the very first thing we do. ========================================== [#2032] lp1716844: Escaping control characters in the audit log Implemented escaping for the JSON output format, and updated the test suite. The other output formats are left unchanged, as they do not have a well defined method for escaping these characters. XML does support control characters in version 1.1, but most tools only understand 1.0, and our output is 1.0. ========================================== [g5]PS-5363 (Merge MySQL 8.0.17): fixed audit_log.audit_log_default_db MTR test case Similarly to what Oracle did in treir fix for Bug #29248047 "5.7 AUDIT PLUGIN DOES NOT LOG WHO UNINSTALLS THE AUDIT PLUGIN UNLIKE 5.6" (commit mysql/mysql-server@6a893b8) added '--source include/disconnect_connections.inc' to 'audit_log.audit_log_default_db' MTR test case after plugin uninstallation.
Blueprint https://blueprints.launchpad.net/percona-server/+spec/audit-log-default-db Add field `DB' for `MYSQL_AUDIT_GENERAL_STATUS' records. ========================================== [mysql#786] Bug 1610242: Reading past the end of heap buffer on saving the default DB in audit p Original title: [mysql#786] Bug 1610242: Reading past the end of heap buffer on saving the default DB in audit plugin memcpy in the call memcpy(local->db, event_general->general_query, sizeof(local->db)); always copies `NAME_LEN' bytes, but the real length of the `general_query' can be less than `NAME_LEN' Fix is to copy `general_query_length' bytes. ========================================== [#930] Bug 1613647: Sporadic audit_log_filter_users failures Insert silent statements (which aren't are grepped away) before connects to make sure the previous statement we which want to see in the audit log actually gets there before the connect. ========================================== [#941] Bug 1617833: audit_log_default_db failures 1. Insert silent statement after UNINSTALL PLUGIN to make sure that SHOW WARNINGS is logged before the statement from the next connection. 2. Insert silent statement before establishing the connection which does 'INSTALL PLUGIN', make sure 'SELECT * FROM t' gets to the log. 3. Make reconnect in case of error optional for 'change_user' mysqltest command ========================================== [#1682] Bug 1650294: Test main.audit_log_filter_users is unstable The source of instability is the fact that event of creating new connection and closing previous one are not synchronized. It makes "Quit" record to appear after "Connect" one. Fix is to replace `include/wait_until_disconnected.inc' which waits for client connection to be closed with `include/wait_until_count_sessions.inc' which waits for specific `Threads_connected' value which is updated after `MYSQL_AUDIT_NOTIFY_CONNECTION_DISCONNECT'. ========================================== [#1762] Bug 1626559: Test `main.audit_log_default_db' is unstable Sometimes "SHOW WARNINGS" appears in the audit log around UNINSTALL PLUGIN / INSTALL PLUGIN. This is the warning which MySQL issue about plugin being in use. Normally warning should not make it to audit log because test case truncates log after UNINSTALL PLUGIN. But sometimes mysql-test picks this warning up after plugin has been reinstalled and it gets logged into audit log. This patch makes sure that test execution continues only when plugin disappeared from the `mysql.plgins', which happens after `deinit' is called and all plugin is fully shut down. ========================================== [mysql#736] Allow to log only queries for specific DBs in audit log plugin Blueprint: [https://blueprints.launchpad.net/percona-server/+spec/audit-filer-db] Add two global variables: - `audit_log_include_databases' comma separated list of databases to include in audit logging - `audit_log_exclude_databases' comma separated list of databases to exclude from audit logging Allow escaped backticks in the `next_word' function. This change needs to be backported to the 5.6. Patch changes behaviour of user and database filters. User filter becomes case sensitive for user name and case insensitive for host name. Database filter becomes case insensitive. [#931] Bug 1617828: audit_log_filter_db failures The fix increases timeout for event shot wait to 10 minutes and adds silent query at the end of the event. ============================================================ [mysql#826] Add filtering by `sql_command' Blueprint: https://blueprints.launchpad.net/percona-server/+spec/audit-filer-command - add option `audit_log_include_command' to specify commands to log - add option `audit_log_exclude_command' to specify commands to exclude from logging - backport changes for `next_word' fixing the capture of default database with quoted backticks. - backport case-sensitivity fix for account filters ========================================== [#1381] Bug 1650321: Valgrind error on main.audit_log_filter_commands Valgrind complained about `cmd' being used uninitialized. However, I don't a path when it could happen, since the first command was to initialize it unconditionally. Still this error message pointed me to the fact that we don't need `cmd' at all, we can simply use `name' and `length' provided as arguments. ========================================== [#1444] Bug 1663251: ASan errors on main.audit_log_filter_commands `my_hash_search' takes two arguments - key to search for and it's length. When length is 0, `my_hash_search' considers that key has the same length as hash length. Since this behaviour may be relied on somewhere, the fix is to avoid calling `my_hash_search' for empty strings. ========================================== [#846] Bug 1614444: Assertion `local->stack.frames[local->stack.top].query == event_general->general_query.str' failed It is a bug which can affects filtering by DB for triggers. There is a trigger CREATE TRIGGER tr1 BEFORE INSERT ON t1 FOR EACH ROW SET @Aux=1; and query INSERT INTO t1(c5,c6)VALUES (1,0); Lets see which events are sent to the audit_log: 1. TABLE ACCESS event for "INSERT INTO t1(c5,c6)VALUES (1,0)" 2. STATUS event for "SET @Aux=1" 3. STATUS event for "INSERT INTO t1(c5,c6)VALUES (1,0)" When (1) comes, plugin pushes "INSERT INTO t1(c5,c6)VALUES (1,0)" to the top of the stack, so it can match it later with STATUS event. When (2) comes, plugin is trying to match "SET @Aux=1" against the stack top. But "SET @Aux=1" isn't on the stack since it doesn't access any database. In debug mode we see the assertion failure. In the release mode, "SET @Aux=1" will be attributed as accessing table "t1", which is wrong. The fix is to replace assert with if statement. Now the query the within trigger which doesn't access any database will always be logged just like any similar query executed outside of the trigger. ============================================================ [#1176] Bug 1641910: Trying to set audit_log_exclude_accounts crashes server The root cause of this bug is that logic around filtering variables rely heavily on the fact that CHECK and UPDATE functions are always called. This is not true when variables passed via defaults file or set as command line options. The fix is add special handling for filtering variables on startup. ========================================== [#1451] Bug 1666496: Server crashes on startup if plugin is unable to create file pointed by --audit_log_file In order to expose this bug we need to set both --audit-log-exclude-commands and --audit-log-file at start up. Second one has to be incorrect, so that plugin would refuse to start with error: mysqld: File './data-dir/mysql-audit.json' not found (Errcode: 2 - No such file or directory) 2017-02-21T11:58:42.392280Z 0 [ERROR] Plugin audit_log reported: 'Cannot open file ./data-dir/mysql-audit.json.' 2017-02-21T11:58:42.392350Z 0 [ERROR] Plugin audit_log reported: 'Error: No such file or directory' 2017-02-21T11:58:42.392355Z 0 [ERROR] Plugin 'audit_log' init function returned error. 2017-02-21T11:58:42.392360Z 0 [ERROR] Plugin 'audit_log' registration as a AUDIT failed. The cause for the crash is common with bug 1641910. MEMALLOC'ed variables are not properly initialized by server (UPDATE and CHECK functions aren't invoked). The fix is to make sure that initialization of MEMALLOC'ed variables is the very first thing we do. ========================================== [#2032] lp1716844: Escaping control characters in the audit log Implemented escaping for the JSON output format, and updated the test suite. The other output formats are left unchanged, as they do not have a well defined method for escaping these characters. XML does support control characters in version 1.1, but most tools only understand 1.0, and our output is 1.0. ========================================== [g5]PS-5363 (Merge MySQL 8.0.17): fixed audit_log.audit_log_default_db MTR test case Similarly to what Oracle did in treir fix for Bug #29248047 "5.7 AUDIT PLUGIN DOES NOT LOG WHO UNINSTALLS THE AUDIT PLUGIN UNLIKE 5.6" (commit mysql/mysql-server@6a893b8) added '--source include/disconnect_connections.inc' to 'audit_log.audit_log_default_db' MTR test case after plugin uninstallation.
Bug#121063: https://bugs.mysql.com/bug.php?id=121063
What happens
Two members of the same group assign different GNOs to the same transaction, in multi-primary mode, when a member leaves and rejoins during a rolling restart under write load. The group splits into internally consistent sets that disagree on the GTID of the same transaction. It can surface on the
group_replication_applierchannel as Error_code 1032 or 1062, or stay silent with every member ONLINE and the disagreement present only in the binary logs.Reproduced on 8.4.8, 8.4.11 and 9.7.2. A self contained reproducer is attached to the bug.
Where it starts
A reserved synode is only ours while we still hold the node index it was reserved under.
local_synode_allocatorstamps the member's current index into the synode,synode.node = my_nodeno. Still insidereserve_synode_number, the task yields in thewhile (too_far(*msgno))loop atTIMED_TASK_WAIT. During that yield,site_install_actionreassignssite->nodeno. The reservation still carries the old index, soproposer_taskbrands and proposes into a slot that now belongs to another node.The header comment of
xcom_base.ccstates the rule at line 107: only node N may propose a value for synode {X N}. With two proposers on the same slot atcnt=0,acceptor.promiseis never raised, since it is assigned in exactly one place, insidehandle_simple_prepare, which is the phase 1 decision. Both proposals are accepted, both are learned, andhandle_learnkeeps whichever LEARN arrived first because of the/* Avoid re-learn */guard. Members that heard different values first deliver different payloads.A bpftrace trace of one such slot followed from ACCEPT through LEARN to DELIVER is in the bug, posted 28 Aug 2026.
The change
Before proposing, verify the reservation still carries the member's own node index. If it does not, drop it through
retry_newand take a new one.This is the same check
incr_msgno(xcom_base.cc:668) already makes whenever it advances, with the commentIn case site and node number has changed. The client transaction is not lost, it goes out in a slot that does belong to the member.Testing
In the lab the anomaly goes to zero: 0 stale proposals out of 15,523,048, against 993 predicted from the unpatched rate, and 0 divergences out of 5,667,504 transactions, against 17.1 expected. No run carrying the patch has reproduced the divergence, on two hosts.
Performance: 12 runs per arm across two hosts, load only, no restarts, no emulated latency, comparing the patched plugin against the same source built without the patch and against the stock image. Ratio of median successful operations per 10 s window, bootstrap over runs: patched over unpatched [0.9998, 1.0004], patched over stock [0.9994, 1.0003]. A positive control with a known throttle was detected at [0.9944, 0.9953], so the method resolves differences of about 0.5%. The workload is rate limited, so this measures behaviour under a production-like load and not peak capacity.
I do not have an MTR test for this. The reproduction needs seven members, rolling restarts and sustained write load, which does not fit the standard framework. The reproducer attached to the bug builds that environment and decides the verdict by decoding every member's binary log and comparing all 21 pairs.