nsivabalan commented on code in PR #6393:
URL: https://github.com/apache/hudi/pull/6393#discussion_r957759152
##########
hudi-common/src/main/java/org/apache/hudi/common/table/view/RemoteHoodieTableFileSystemView.java:
##########
@@ -495,4 +496,14 @@ public Option<HoodieBaseFile> getLatestBaseFile(String
partitionPath, String fil
throw new HoodieRemoteException(e);
}
}
+
+ public static Response get(int timeoutMs, String url, RequestMethod method)
throws IOException {
Review Comment:
does this have to be public ?
##########
hudi-timeline-service/src/test/java/org/apache/hudi/timeline/service/functional/TestRemoteHoodieTableFileSystemView.java:
##########
@@ -64,4 +66,31 @@ protected SyncableFileSystemView
getFileSystemView(HoodieTimeline timeline) {
view = new RemoteHoodieTableFileSystemView("localhost",
server.getServerPort(), metaClient);
return view;
}
+
+ @Test
+ public void testRemoteHoodieTableFileSystemViewWithRetry() {
+ // Service is available.
+ view.getLatestBaseFiles();
+ // Shut down the service.
+ server.close();
+ try {
+ // Immediately fails and throws a connection refused exception.
+ view.getLatestBaseFiles();
+ } catch (HoodieRemoteException e) {
Review Comment:
don't we need to add Assert.fail() just after line 78?
##########
hudi-timeline-service/src/test/java/org/apache/hudi/timeline/service/functional/TestRemoteHoodieTableFileSystemView.java:
##########
@@ -64,4 +66,31 @@ protected SyncableFileSystemView
getFileSystemView(HoodieTimeline timeline) {
view = new RemoteHoodieTableFileSystemView("localhost",
server.getServerPort(), metaClient);
return view;
}
+
+ @Test
+ public void testRemoteHoodieTableFileSystemViewWithRetry() {
+ // Service is available.
+ view.getLatestBaseFiles();
+ // Shut down the service.
+ server.close();
+ try {
+ // Immediately fails and throws a connection refused exception.
+ view.getLatestBaseFiles();
+ } catch (HoodieRemoteException e) {
+ assert e.getMessage().contains("Connection refused (Connection
refused)");
+ }
+ // Enable API request retry for remote file system view.
+ view = new RemoteHoodieTableFileSystemView(metaClient,
FileSystemViewStorageConfig
+ .newBuilder()
+ .withRemoteServerHost("localhost")
+ .withRemoteServerPort(server.getServerPort())
+ .withRemoteTimelineClientRetry(true)
+ .withRemoteTimelineClientMaxRetryNumbers(4)
+ .build());
+ try {
+ view.getLatestBaseFiles();
Review Comment:
don't we need to add Assert.fail() just after line 91?
##########
hudi-timeline-service/src/test/java/org/apache/hudi/timeline/service/functional/TestRemoteHoodieTableFileSystemView.java:
##########
@@ -64,4 +66,31 @@ protected SyncableFileSystemView
getFileSystemView(HoodieTimeline timeline) {
view = new RemoteHoodieTableFileSystemView("localhost",
server.getServerPort(), metaClient);
return view;
}
+
+ @Test
+ public void testRemoteHoodieTableFileSystemViewWithRetry() {
+ // Service is available.
+ view.getLatestBaseFiles();
+ // Shut down the service.
+ server.close();
+ try {
+ // Immediately fails and throws a connection refused exception.
+ view.getLatestBaseFiles();
+ } catch (HoodieRemoteException e) {
+ assert e.getMessage().contains("Connection refused (Connection
refused)");
+ }
+ // Enable API request retry for remote file system view.
+ view = new RemoteHoodieTableFileSystemView(metaClient,
FileSystemViewStorageConfig
+ .newBuilder()
+ .withRemoteServerHost("localhost")
+ .withRemoteServerPort(server.getServerPort())
+ .withRemoteTimelineClientRetry(true)
+ .withRemoteTimelineClientMaxRetryNumbers(4)
+ .build());
+ try {
+ view.getLatestBaseFiles();
Review Comment:
is it no possible to test that retry succeed after 2 or 3 tries?
--
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]
