findingrish commented on code in PR #14468:
URL: https://github.com/apache/druid/pull/14468#discussion_r1244089296
##########
server/src/test/java/org/apache/druid/server/coordination/ChangeRequestHttpSyncerTest.java:
##########
@@ -22,106 +22,45 @@
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
-import com.google.common.util.concurrent.Futures;
-import com.google.common.util.concurrent.ListenableFuture;
-import org.apache.druid.java.util.common.RE;
+import org.apache.druid.client.TestSegmentChangeRequestHttpClient;
+import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.concurrent.Execs;
-import org.apache.druid.java.util.http.client.HttpClient;
-import org.apache.druid.java.util.http.client.Request;
-import org.apache.druid.java.util.http.client.response.HttpResponseHandler;
import org.apache.druid.segment.TestHelper;
import org.easymock.EasyMock;
-import org.jboss.netty.buffer.ChannelBuffers;
-import org.jboss.netty.handler.codec.http.DefaultHttpResponse;
-import org.jboss.netty.handler.codec.http.HttpResponse;
-import org.jboss.netty.handler.codec.http.HttpResponseStatus;
-import org.jboss.netty.handler.codec.http.HttpVersion;
import org.joda.time.Duration;
import org.junit.Test;
-import java.io.ByteArrayInputStream;
import java.net.URL;
-import java.util.List;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.Arrays;
/**
+ *
Review Comment:
nit: we can remove empty comment block
##########
processing/src/test/java/org/apache/druid/java/util/common/DateTimesTest.java:
##########
@@ -123,4 +126,28 @@ public void testCanCompareAsString()
DateTimes.of("2000").withZone(DateTimes.inferTzFromString("America/Los_Angeles")))
);
}
+
+ @Test
+ public void testMillisElapsed() throws Exception
+ {
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ Thread.sleep(100);
+ stopwatch.stop();
+
+ Assert.assertEquals(
+ stopwatch.elapsed(TimeUnit.MILLISECONDS),
+ DateTimes.millisElapsed(stopwatch)
+ );
+ }
+
+ @Test
+ public void testHasElapsed() throws Exception
+ {
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ Thread.sleep(100);
Review Comment:
same comment as above.
##########
processing/src/test/java/org/apache/druid/java/util/common/DateTimesTest.java:
##########
@@ -123,4 +126,28 @@ public void testCanCompareAsString()
DateTimes.of("2000").withZone(DateTimes.inferTzFromString("America/Los_Angeles")))
);
}
+
+ @Test
+ public void testMillisElapsed() throws Exception
+ {
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ Thread.sleep(100);
Review Comment:
Instead of sleep, we can use a custom ticker to advance time artificially.
`FakeTicker` is a subclass which can be used to
```
FakeTicker fakeTicker = new FakeTicker();
Stopwatch stopwatch = Stopwatch.createStarted(fakeTicker);
fakeTicker.advance(100, TimeUnit.MILLIS);
```
##########
server/src/test/java/org/apache/druid/client/HttpServerInventoryViewTest.java:
##########
@@ -305,42 +311,28 @@ public ServerView.CallbackAction
serverRemoved(DruidServer server)
serverRemovedCalled.await();
Assert.assertNull(httpServerInventoryView.getInventoryValue("host:8080"));
- EasyMock.verify(druidNodeDiscoveryProvider);
-
httpServerInventoryView.stop();
}
@Test(timeout = 60_000L)
public void testSyncMonitoring()
{
- ObjectMapper jsonMapper = TestHelper.makeJsonMapper();
-
- TestDruidNodeDiscovery druidNodeDiscovery = new TestDruidNodeDiscovery();
- DruidNodeDiscoveryProvider druidNodeDiscoveryProvider =
EasyMock.createMock(DruidNodeDiscoveryProvider.class);
-
EasyMock.expect(druidNodeDiscoveryProvider.getForService(DataNodeService.DISCOVERY_SERVICE_KEY))
- .andReturn(druidNodeDiscovery);
- EasyMock.replay(druidNodeDiscoveryProvider);
-
- TestHttpClient httpClient = new TestHttpClient(ImmutableList.of());
-
- HttpServerInventoryView httpServerInventoryView = new
HttpServerInventoryView(
- jsonMapper,
- httpClient,
- druidNodeDiscoveryProvider,
- (pair) -> !pair.rhs.getDataSource().equals("non-loading-datasource"),
- new HttpServerInventoryViewConfig(null, null, null),
- "test"
- );
-
httpServerInventoryView.start();
httpServerInventoryView.serverAdded(makeServer("abc.com:8080"));
httpServerInventoryView.serverAdded(makeServer("xyz.com:8080"));
httpServerInventoryView.serverAdded(makeServer("lol.com:8080"));
Assert.assertEquals(3, httpServerInventoryView.getDebugInfo().size());
- httpServerInventoryView.syncMonitoring();
+ httpServerInventoryView.checkAndResetUnhealthyServers();
Assert.assertEquals(3, httpServerInventoryView.getDebugInfo().size());
}
+ @Test
+ @Ignore
+ public void testSyncStatusMetricsAreEmitted()
Review Comment:
should this be removed?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]