[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471: URL: https://github.com/apache/flink/pull/22471#issuecomment-1521416377 Thanks @zentol and @reswqa for the quick reviews. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471: URL: https://github.com/apache/flink/pull/22471#issuecomment-1520356264 @zentol Thanks for the quick replies and ideas. Since no real servers are created based on the server address, I think the given fixed port is enough. And the hostname has been updated. Could you please help take a look again? Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471: URL: https://github.com/apache/flink/pull/22471#issuecomment-1520237800 I agree that using port 0 is usually the best approach to resolve conflicts, but it indeed does not work well in this test. > The UDP socket only ensures no other process binds to that port in the mean time. I don't think so. Because TCP and UDP can use the same port at the same time. For example, if the UDP server has used port 23456, the TCP can also use the same port at the same time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471: URL: https://github.com/apache/flink/pull/22471#issuecomment-1520008996 @zentol Apologies for my lack of understanding, but I am unclear on how the change ensures that the port is available. When creating a `DatagramSocket s = new DatagramSocket()`, a UDP socket is generated. However, when an `InetSocketAddress` is created, it only specifies an **Integer** type of the port. How does the `InetSocketAddress` differentiate between UDP and TCP? In my opinion, if we do not change the client's initialization process, `client.sendRequest` will still utilize TCP. Therefore, I fail to see how using `new DatagramSocket()` versus `new ServerSocket` makes a difference. Could you help take a look again? Thanks a lot. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471:
URL: https://github.com/apache/flink/pull/22471#issuecomment-1519868580
@zentol Thanks for joining the review.
`InetSocketAddress` only uses a random port when the **`bind`** operation
occurs, as seen in the InetSocketAddress docs.
But in this test, it does not trigger the `bind` operation obviously, so it
uses port 0 directly and causes the test to be unstable when port 0 is
occupied. I tested locally and the test often fails because of the
`BindException`.
If you do not agree on the fix, maybe another fixing method is that use the
`bind` to trigger to choose an available port like this to replace the line
`InetSocketAddress address = new InetSocketAddress(InetAddress.getLocalHost(),
0)`.
```
ServerSocket serverSocket = new ServerSocket();
InetSocketAddress address = new
InetSocketAddress(InetAddress.getLocalHost(), 0);
System.out.println("The port 1 " + address.getPort());
serverSocket.bind(address);
serverSocket.close();
System.out.println("The port 2 " + serverSocket.getLocalPort());
InetSocketAddress serverAddress =
new InetSocketAddress(InetAddress.getLocalHost(),
serverSocket.getLocalPort());
```
(These `System.out.println` should be removed. They are only used to see the
results.)
You can see that `port 1` is 0 and `port 2` is another available port. Then
the test will be stable.
WDYT about the new fixing method?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471: URL: https://github.com/apache/flink/pull/22471#issuecomment-1519651340 @reswqa Thanks for the quick review and the quick reply. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] TanYuxin-tyx commented on pull request #22471: [FLINK-31897] Fix the unstable test ClientTest#testRequestUnavailableHost
TanYuxin-tyx commented on PR #22471: URL: https://github.com/apache/flink/pull/22471#issuecomment-1519482485 Thanks for helping review, @reswqa I have addressed the comments. Please take a look again, thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
