yihua commented on code in PR #5884:
URL: https://github.com/apache/hudi/pull/5884#discussion_r955364712
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/RemoteHoodieTableFileSystemView.java:
##########
@@ -165,17 +179,9 @@ private <T> T executeRequest(String requestPath,
Map<String, String> queryParame
String url = builder.toString();
LOG.info("Sending request : (" + url + ")");
- Response response;
- int timeout = this.timeoutSecs * 1000; // msec
- switch (method) {
- case GET:
- response =
Request.Get(url).connectTimeout(timeout).socketTimeout(timeout).execute();
- break;
- case POST:
- default:
- response =
Request.Post(url).connectTimeout(timeout).socketTimeout(timeout).execute();
- break;
- }
+ // Reset url and method, to avoid repeatedly instantiating objects.
+ urlCheckedFunc.setUrlAndMethod(url, method);
+ Response response = retryHelper != null ?
retryHelper.tryWith(urlCheckedFunc).start() : urlCheckedFunc.get();
Review Comment:
@LinMingQiang @danny0405 Every request goes through this flow and the
`urlCheckedFunc` should not be shared across requests. The logic here is
incorrect. This can cause serious correctness problems under concurrency.
We need to revert this logic. Also suggest that we should guard against
such changes in the hot path with a flag.
cc @nsivabalan @rmahindra123
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
