fix wildcard interconnect address type #936
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
| wait_for_receiver(false); | ||
| } | ||
|
|
||
|
|
||
| static void | ||
| test_send_dummy_packet_ipv6_to_ipv6_wildcard(void **state) |
There was a problem hiding this comment.
the function is called test_send_dummy_packet_ipv6_to_ipv6_wildcard, but the test sets
Gp_interconnect_address_type to INTERCONNECT_ADDRESS_TYPE_UNICAST.
There was a problem hiding this comment.
Generally, this patch didn't use to touch existing test cases and only make default value explicit. The default value of Gp_interconnect_address_type is unicast so this "wildcard" test tested something on the unicast configuration. But as far as this test fully used to duplicate by test_send_dummy_packet_ipv6_to_ipv6, I took in account your notice and replaced it with wildcard one. On Linux machines getaddrinfo returns ipv4 as a first item for wildcard request even on machines with ipv6 support, so this test is identical to ipv6_to_ipv4 now, but pin this implicit Linux behavior.
There was a problem hiding this comment.
Maybe we should rename the test to test_send_dummy_packet_ipv6_to_ipv4 then? This seems confusing
There was a problem hiding this comment.
Why not test_send_dummy_packet_ipv4_to_ipv4? As far as receiving and sending sockets will use ipv4 despite of presence IPv6 in case of wildcard.
In my opinion, we pin fallback to IPv4 on Linux by this test.
There was a problem hiding this comment.
Isn't the sender socket explicitly AF_INET6? I think wildcard only affects the receiver socket in this test. And sorry, I meant test_send_dummy_packet_ipv6_to_ipv6_wildcard -> test_send_dummy_packet_ipv6_to_ipv4_wildcard
Partially revert 50748b7 changes that broke down wildcard interconnect address type handling for gpdb 7. Removed duplicated assertion, already checked for unicast interconnect few lines above. interconnect_address is null for wildcard address type. In this case service arg of getaddrinfo must not be null, because either node or service may be null according to man page. A unit test for this case provided too.
| wait_for_receiver(false); | ||
| } | ||
|
|
||
|
|
||
| static void | ||
| test_send_dummy_packet_ipv6_to_ipv6_wildcard(void **state) |
There was a problem hiding this comment.
Maybe we should rename the test to test_send_dummy_packet_ipv6_to_ipv4 then? This seems confusing
| @@ -317,6 +353,7 @@ main(int argc, char* argv[]) | |||
| unit_test(test_send_dummy_packet_ipv4_to_ipv6_should_fail), | |||
| unit_test(test_send_dummy_packet_ipv6_to_ipv6), | |||
| unit_test(test_send_dummy_packet_ipv6_to_ipv4), | |||
| unit_test(test_send_dummy_packet_ipv4_to_ipv4_wildcard), | |||
There was a problem hiding this comment.
Since this doesn't require IPv6, we can add it to else branch too.
|
Please start the summary with a capital letter: |
|
In the description:
|
| @@ -1185,6 +1186,7 @@ setupUDPListeningSocket(int *listenerSocketFd, uint16 *listenerPort, int *txFami | |||
| uint32 socketSendBufferSize; | |||
| uint32 socketRecvBufferSize; | |||
|
|
|||
| snprintf(service, 32, "%d", 0); | |||
There was a problem hiding this comment.
setupTCPListeningSocket() contains a comment about similar line. Maybe add here the same comment as well?
Fix wildcard interconnect address type
GPDB used to fail to setup UDP interconnect sockets when
gp_interconnect_address_type GUC was set to wildcard mode. The problem was caused
by passing invalid set of arguments to getaddinfo library function and
consequent getting EAI_NONAME errno. interconnect_address is null for wildcard
address type. In this case service arg of getaddrinfo must not be null, because
either node or service may be null according to man page of getaddrinfo(3).
The mentioned problem was caused by 50748b7. So partially reverted this patch.
Removed duplicated assertion, already checked for unicast interconnect few
lines above.
A unit test for this case is provided too. The default interconnect address
type was explicitly point for existing tests. IPv6 wildcard test was fully
duplicated by existing unicast test and was reworked to test wildcard
configuration. It's expected that Linux returns IPv4 address first for wildcard
request even on machines with IPv6. So, pined it by this test.