From 24fcc00214bd43e07f7c4cc76362c1669f82ddc4 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Wed, 19 Mar 2025 15:57:34 +0100 Subject: [PATCH 01/16] fix OOM error in test --- .../src/test/java/com/segment/analytics/AnalyticsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java b/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java index 8be3012e..ca3a8099 100644 --- a/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java +++ b/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Before; import org.junit.Test; @@ -143,7 +144,7 @@ public void threadSafeTest(MessageBuilderTest builder) } while (initialTime.until(now, ChronoUnit.MILLIS) < millisRunning); service.shutdown(); - while (!service.isShutdown() || !service.isTerminated()) {} + service.awaitTermination(5, TimeUnit.SECONDS); verify(spy, times(counter.get())).enqueue(any(Message.class)); } From 8d814200660a2dd8ead6005749fc79e988c86045 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Thu, 20 Mar 2025 16:12:59 +0100 Subject: [PATCH 02/16] setup wiremock test --- analytics/pom.xml | 7 +++- .../com/segment/analytics/SegmentTest.java | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 analytics/src/test/java/com/segment/analytics/SegmentTest.java diff --git a/analytics/pom.xml b/analytics/pom.xml index 8c80856c..bf2e9ae1 100644 --- a/analytics/pom.xml +++ b/analytics/pom.xml @@ -67,7 +67,12 @@ org.mockito mockito-core test - + + + org.wiremock + wiremock-standalone + 3.2.0 + diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java new file mode 100644 index 00000000..531229d3 --- /dev/null +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -0,0 +1,40 @@ +package com.segment.analytics; + +import static com.github.tomakehurst.wiremock.client.WireMock.okJson; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.segment.analytics.messages.TrackMessage; +import java.util.UUID; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +public class SegmentTest { + + @Rule + public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort(), false); + + Analytics analytics; + + @Before + public void confWireMock() { + stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); + + analytics = Analytics.builder("write-key") + .endpoint(wireMockRule.baseUrl()) + // callback + // http client + .build(); + } + + @Test + public void test() { + analytics.enqueue(TrackMessage.builder("my-track") + .messageId(UUID.randomUUID().toString()) + .userId("userId")); + } +} From 3d232fadb3f9d87446c2ef3c5ef0335100f4422b Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Thu, 20 Mar 2025 18:16:22 +0100 Subject: [PATCH 03/16] remove --- .../java/com/segment/analytics/Analytics.java | 5 - .../analytics/internal/AnalyticsClient.java | 117 +++++------------- .../com/segment/analytics/AnalyticsTest.java | 7 -- .../internal/AnalyticsClientTest.java | 48 +------ 4 files changed, 35 insertions(+), 142 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java index 81af36c7..26bd0bcb 100644 --- a/analytics/src/main/java/com/segment/analytics/Analytics.java +++ b/analytics/src/main/java/com/segment/analytics/Analytics.java @@ -90,11 +90,6 @@ public boolean offer(MessageBuilder builder) { return client.offer(message); } - /** Flush events in the message queue. */ - public void flush() { - client.flush(); - } - /** Stops this instance from processing further requests. */ public void shutdown() { client.shutdown(); diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index f7560004..024f2186 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -24,9 +24,7 @@ import java.util.UUID; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -54,17 +52,17 @@ public class AnalyticsClient { } private final BlockingQueue messageQueue; + private final BlockingQueue pendingQueue; private final HttpUrl uploadUrl; private final SegmentService service; private final int size; + private final long flushIntervalInMillis; private final int maximumRetries; private final int maximumQueueByteSize; - private int currentQueueSizeInBytes; private final Log log; private final List callbacks; private final ExecutorService networkExecutor; - private final ExecutorService looperExecutor; - private final ScheduledExecutorService flushScheduler; + private final Thread looperThread; private final AtomicBoolean isShutDown; private final String writeKey; @@ -83,6 +81,7 @@ public static AnalyticsClient create( String writeKey, Gson gsonInstance) { return new AnalyticsClient( + new LinkedBlockingQueue(queueCapacity), new LinkedBlockingQueue(queueCapacity), uploadUrl, segmentService, @@ -101,6 +100,7 @@ public static AnalyticsClient create( public AnalyticsClient( BlockingQueue messageQueue, + BlockingQueue pendingQueue, HttpUrl uploadUrl, SegmentService service, int maxQueueSize, @@ -115,34 +115,20 @@ public AnalyticsClient( String writeKey, Gson gsonInstance) { this.messageQueue = messageQueue; + this.pendingQueue = pendingQueue; this.uploadUrl = uploadUrl; this.service = service; this.size = maxQueueSize; + this.flushIntervalInMillis = flushIntervalInMillis; this.maximumRetries = maximumRetries; this.maximumQueueByteSize = maximumQueueSizeInBytes; this.log = log; this.callbacks = callbacks; - this.looperExecutor = Executors.newSingleThreadExecutor(threadFactory); + this.looperThread = threadFactory.newThread(new Looper()); this.networkExecutor = networkExecutor; this.isShutDown = isShutDown; this.writeKey = writeKey; this.gsonInstance = gsonInstance; - - this.currentQueueSizeInBytes = 0; - - if (!isShutDown.get()) looperExecutor.submit(new Looper()); - - flushScheduler = Executors.newScheduledThreadPool(1, threadFactory); - flushScheduler.scheduleAtFixedRate( - new Runnable() { - @Override - public void run() { - flush(); - } - }, - flushIntervalInMillis, - flushIntervalInMillis, - TimeUnit.MILLISECONDS); } public int messageSizeInBytes(Message message) { @@ -151,74 +137,33 @@ public int messageSizeInBytes(Message message) { return stringifiedMessage.getBytes(ENCODING).length; } - private Boolean isBackPressuredAfterSize(int incomingSize) { - int POISON_BYTE_SIZE = messageSizeInBytes(FlushMessage.POISON); - int sizeAfterAdd = this.currentQueueSizeInBytes + incomingSize + POISON_BYTE_SIZE; - // Leave a 10% buffer since the unsynchronized enqueue could add multiple at a time - return sizeAfterAdd >= Math.min(this.maximumQueueByteSize, BATCH_MAX_SIZE) * 0.9; - } - public boolean offer(Message message) { return messageQueue.offer(message); } - public void enqueue(Message message) { - if (message != StopMessage.STOP && isShutDown.get()) { + public void enqueue(Message message) {} + + public void enqueueSend(Message message) { + if (isShutDown.get()) { log.print(ERROR, "Attempt to enqueue a message when shutdown has been called %s.", message); return; } try { - // @jorgen25 message here could be regular msg, POISON or STOP. Only do regular logic if its - // valid message - if (message != StopMessage.STOP && message != FlushMessage.POISON) { - int messageByteSize = messageSizeInBytes(message); - - // @jorgen25 check if message is below 32kb limit for individual messages, no need to check - // for extra characters - if (messageByteSize <= MSG_MAX_SIZE) { - if (isBackPressuredAfterSize(messageByteSize)) { - this.currentQueueSizeInBytes = messageByteSize; - messageQueue.put(FlushMessage.POISON); - messageQueue.put(message); - - log.print(VERBOSE, "Maximum storage size has been hit Flushing..."); - } else { - messageQueue.put(message); - this.currentQueueSizeInBytes += messageByteSize; - } - } else { - log.print( - ERROR, "Message was above individual limit. MessageId: %s", message.messageId()); - throw new IllegalArgumentException( - "Message was above individual limit. MessageId: " + message.messageId()); - } - } else { - messageQueue.put(message); - } + messageQueue.put(message); } catch (InterruptedException e) { log.print(ERROR, e, "Interrupted while adding message %s.", message); Thread.currentThread().interrupt(); } } - public void flush() { - if (!isShutDown.get()) { - enqueue(FlushMessage.POISON); - } - } - public void shutdown() { if (isShutDown.compareAndSet(false, true)) { final long start = System.currentTimeMillis(); // first let's tell the system to stop - enqueue(StopMessage.STOP); - - // we can shutdown the flush scheduler without worrying - flushScheduler.shutdownNow(); + looperThread.interrupt(); - shutdownAndWait(looperExecutor, "looper"); shutdownAndWait(networkExecutor, "network"); log.print( @@ -247,30 +192,21 @@ public void shutdownAndWait(ExecutorService executor, String name) { * messages, it triggers a flush. */ class Looper implements Runnable { - private boolean stop; public Looper() { - this.stop = false; } @Override public void run() { LinkedList messages = new LinkedList<>(); - AtomicInteger currentBatchSize = new AtomicInteger(); + int currentBatchSize = 0; boolean batchSizeLimitReached = false; int contextSize = gsonInstance.toJson(CONTEXT).getBytes(ENCODING).length; try { - while (!stop) { - Message message = messageQueue.take(); - - if (message == StopMessage.STOP) { - log.print(VERBOSE, "Stopping the Looper"); - stop = true; - } else if (message == FlushMessage.POISON) { - if (!messages.isEmpty()) { - log.print(VERBOSE, "Flushing messages."); - } - } else { + while (!Thread.currentThread().isInterrupted()) { + Message message = messageQueue.poll(flushIntervalInMillis, TimeUnit.MILLISECONDS); + + if (message != null) { // we do +1 because we are accounting for this new message we just took from the queue // which is not in list yet // need to check if this message is going to make us go over the limit considering @@ -278,9 +214,9 @@ public void run() { int defaultBatchSize = BatchUtility.getBatchDefaultSize(contextSize, messages.size() + 1); int msgSize = messageSizeInBytes(message); - if (currentBatchSize.get() + msgSize + defaultBatchSize <= BATCH_MAX_SIZE) { + if (currentBatchSize + msgSize + defaultBatchSize <= BATCH_MAX_SIZE) { messages.add(message); - currentBatchSize.addAndGet(msgSize); + currentBatchSize+=msgSize; } else { // put message that did not make the cut this time back on the queue, we already took // this message if we dont put it back its lost @@ -288,8 +224,12 @@ public void run() { batchSizeLimitReached = true; } } + + if (messages.isEmpty()) { + continue; + } - Boolean isBlockingSignal = message == FlushMessage.POISON || message == StopMessage.STOP; + Boolean isBlockingSignal = message == null; Boolean isOverflow = messages.size() >= size; if (!messages.isEmpty() && (isOverflow || isBlockingSignal || batchSizeLimitReached)) { @@ -302,7 +242,7 @@ public void run() { networkExecutor.submit( BatchUploadTask.create(AnalyticsClient.this, batch, maximumRetries)); - currentBatchSize.set(0); + currentBatchSize=0; messages.clear(); if (batchSizeLimitReached) { // If this is true that means the last message that would make us go over the limit @@ -315,8 +255,9 @@ public void run() { } } catch (InterruptedException e) { log.print(DEBUG, "Looper interrupted while polling for messages."); - Thread.currentThread().interrupt(); + Thread.currentThread().interrupt(); //XXX } + // SEND pending log.print(VERBOSE, "Looper stopped"); } } diff --git a/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java b/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java index ca3a8099..075ae003 100644 --- a/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java +++ b/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java @@ -91,13 +91,6 @@ public void shutdownIsDispatched() { verify(client).shutdown(); } - @Test - public void flushIsDispatched() { - analytics.flush(); - - verify(client).flush(); - } - @Test public void offerIsDispatched(MessageBuilderTest builder) { MessageBuilder messageBuilder = builder.get().userId("dummy"); diff --git a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java b/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java index 74f04e13..b58ec35b 100644 --- a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java +++ b/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java @@ -75,6 +75,7 @@ public class AnalyticsClientTest { ThreadFactory threadFactory; @Spy LinkedBlockingQueue messageQueue; + @Spy LinkedBlockingQueue pendingQueue; @Mock SegmentService segmentService; @Mock ExecutorService networkExecutor; @Mock Callback callback; @@ -95,6 +96,7 @@ public void setUp() { AnalyticsClient newClient() { return new AnalyticsClient( messageQueue, + pendingQueue, null, segmentService, 50, @@ -131,15 +133,6 @@ public void shutdown() throws InterruptedException { verify(networkExecutor).awaitTermination(1, TimeUnit.SECONDS); } - @Test - public void flushInsertsPoison() throws InterruptedException { - AnalyticsClient client = newClient(); - - client.flush(); - - verify(messageQueue).put(FlushMessage.POISON); - } - /** Wait until the queue is drained. */ static void wait(Queue queue) { // noinspection StatementWithEmptyBody @@ -198,21 +191,6 @@ private static String generateDataOfSizeSpecialChars( return builder.toString(); } - @Test - public void flushSubmitsToExecutor() { - messageQueue = new LinkedBlockingQueue<>(); - AnalyticsClient client = newClient(); - - TrackMessage first = TrackMessage.builder("foo").userId("bar").build(); - TrackMessage second = TrackMessage.builder("qaz").userId("qux").build(); - client.enqueue(first); - client.enqueue(second); - client.flush(); - wait(messageQueue); - - assertThat(captureBatch(networkExecutor).batch()).containsExactly(first, second); - } - @Test public void enqueueMaxTriggersFlush() { messageQueue = new LinkedBlockingQueue<>(); @@ -284,6 +262,7 @@ public void flushHowManyTimesNecessaryToStayWithinLimit() throws InterruptedExce AnalyticsClient client = new AnalyticsClient( messageQueue, + pendingQueue, null, segmentService, 50, @@ -544,24 +523,6 @@ public boolean matches(IOException exception) { })); } - @Test - public void flushWhenNotShutDown() throws InterruptedException { - AnalyticsClient client = newClient(); - - client.flush(); - verify(messageQueue).put(POISON); - } - - @Test - public void flushWhenShutDown() throws InterruptedException { - AnalyticsClient client = newClient(); - isShutDown.set(true); - - client.flush(); - - verify(messageQueue, times(0)).put(any(Message.class)); - } - @Test public void enqueueWithRegularMessageWhenNotShutdown(MessageBuilderTest builder) throws InterruptedException { @@ -860,6 +821,7 @@ public void submitBatchBelowThreshold() throws InterruptedException, IllegalArgu AnalyticsClient client = new AnalyticsClient( messageQueue, + pendingQueue, null, segmentService, 50, @@ -902,6 +864,7 @@ public void submitBatchAboveThreshold() throws InterruptedException, IllegalArgu AnalyticsClient client = new AnalyticsClient( messageQueue, + pendingQueue, null, segmentService, 50, @@ -937,6 +900,7 @@ public void submitManySmallMessagesBatchAboveThreshold() throws InterruptedExcep AnalyticsClient client = new AnalyticsClient( messageQueue, + pendingQueue, null, segmentService, 50, From 82fb90840e23b2ff6112667ca23b093d196536df Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Fri, 21 Mar 2025 16:40:00 +0100 Subject: [PATCH 04/16] simple wiremock test --- analytics/pom.xml | 7 ++ .../analytics/internal/AnalyticsClient.java | 9 ++- .../com/segment/analytics/SegmentTest.java | 78 +++++++++++++++++-- 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/analytics/pom.xml b/analytics/pom.xml index bf2e9ae1..6a97d231 100644 --- a/analytics/pom.xml +++ b/analytics/pom.xml @@ -72,7 +72,14 @@ org.wiremock wiremock-standalone 3.2.0 + test + + org.awaitility + awaitility + 4.2.2 + test + diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index 024f2186..1a644aa5 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -28,7 +28,6 @@ import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import okhttp3.HttpUrl; import retrofit2.Call; import retrofit2.Response; @@ -129,6 +128,7 @@ public AnalyticsClient( this.isShutDown = isShutDown; this.writeKey = writeKey; this.gsonInstance = gsonInstance; + looperThread.start(); } public int messageSizeInBytes(Message message) { @@ -141,7 +141,10 @@ public boolean offer(Message message) { return messageQueue.offer(message); } - public void enqueue(Message message) {} + public void enqueue(Message message) { + + enqueueSend(message); + } public void enqueueSend(Message message) { if (isShutDown.get()) { @@ -289,7 +292,7 @@ static BatchUploadTask create(AnalyticsClient client, Batch batch, int maxRetrie private void notifyCallbacksWithException(Batch batch, Exception exception) { for (Message message : batch.batch()) { for (Callback callback : client.callbacks) { - callback.failure(message, exception); + callback.failure(message, exception); } } } diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index 531229d3..5af61174 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -7,34 +7,100 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.github.tomakehurst.wiremock.stubbing.ServeEvent; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.segment.analytics.gson.AutoValueAdapterFactory; +import com.segment.analytics.gson.ISO8601DateAdapter; import com.segment.analytics.messages.TrackMessage; -import java.util.UUID; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.awaitility.Awaitility; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import wiremock.com.fasterxml.jackson.core.JsonProcessingException; +import wiremock.com.fasterxml.jackson.databind.JsonNode; +import wiremock.com.fasterxml.jackson.databind.ObjectMapper; public class SegmentTest { @Rule - public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort(), false); + public WireMockRule wireMockRule = + new WireMockRule(wireMockConfig().dynamicPort().gzipDisabled(true), false); Analytics analytics; + GsonBuilder gsonBuilder = new GsonBuilder() + .registerTypeAdapterFactory(new AutoValueAdapterFactory()) + .registerTypeAdapter(Date.class, new ISO8601DateAdapter()); + + Gson gson = gsonBuilder.create(); + @Before public void confWireMock() { stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); analytics = Analytics.builder("write-key") .endpoint(wireMockRule.baseUrl()) + .flushInterval(1, TimeUnit.SECONDS) + .queueCapacity(500) // callback // http client .build(); } @Test - public void test() { - analytics.enqueue(TrackMessage.builder("my-track") - .messageId(UUID.randomUUID().toString()) - .userId("userId")); + public void test() throws Throwable { + analytics.enqueue(TrackMessage.builder("my-track").messageId("m1").userId("userId")); + analytics.enqueue(TrackMessage.builder("my-track").messageId("m2").userId("userId")); + + Awaitility.await().until(() -> sentMessagesEqualsTo("m1", "m2")); + } + + @Test + public void testMore() throws Throwable { + System.err.println("wm at " + wireMockRule.baseUrl()); + int num = 100_000; + String[] expectedIds = new String[num]; + for (int i = 0; i < num; i++) { + String id = "m" + i; + expectedIds[i] = id; + analytics.enqueue(TrackMessage.builder("my-track").messageId(id).userId("userId")); + } + + Awaitility.await().atMost(1, TimeUnit.MINUTES).until(() -> sentMessagesEqualsTo(expectedIds)); + } + + private static final ObjectMapper OM = new ObjectMapper(); + + private boolean sentMessagesEqualsTo(String... msgIds) { + return new HashSet<>(sentMessages()).equals(new HashSet<>(Arrays.asList(msgIds))); + } + + private List sentMessages() { + List messageIds = new ArrayList<>(); + for (ServeEvent event : wireMockRule.getAllServeEvents()) { + JsonNode batch; + try { + JsonNode json = OM.readTree(event.getRequest().getBodyAsString()); + batch = json.get("batch"); + if (batch == null) { + continue; + } + } catch (JsonProcessingException e) { + continue; + } + Iterator msgs = batch.elements(); + while (msgs.hasNext()) { + messageIds.add(msgs.next().get("messageId").asText()); + } + } + return messageIds; } } From 205e2dee5fa36c06a5bab76252fb133d46c05ce7 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 24 Mar 2025 17:32:46 +0100 Subject: [PATCH 05/16] remove callback --- .../java/com/segment/analytics/Analytics.java | 22 ---- .../analytics/internal/AnalyticsClient.java | 20 +--- .../analytics/AnalyticsBuilderTest.java | 34 ------ .../internal/AnalyticsClientTest.java | 102 ++++++++---------- 4 files changed, 48 insertions(+), 130 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java index 26bd0bcb..d7444643 100644 --- a/analytics/src/main/java/com/segment/analytics/Analytics.java +++ b/analytics/src/main/java/com/segment/analytics/Analytics.java @@ -141,7 +141,6 @@ public static class Builder { private int maximumFlushAttempts; private int maximumQueueSizeInBytes; private long flushIntervalInMillis; - private List callbacks; private int queueCapacity; private boolean forceTlsV1 = false; private GsonBuilder gsonBuilder; @@ -318,21 +317,6 @@ public Builder threadFactory(ThreadFactory threadFactory) { return this; } - /** Add a {@link Callback} to be notified when an event is processed. */ - public Builder callback(Callback callback) { - if (callback == null) { - throw new NullPointerException("Null callback"); - } - if (callbacks == null) { - callbacks = new ArrayList<>(); - } - if (callbacks.contains(callback)) { - throw new IllegalStateException("Callback is already registered."); - } - callbacks.add(callback); - return this; - } - /** Use a {@link Plugin} to configure the builder. */ @Beta public Builder plugin(Plugin plugin) { @@ -407,11 +391,6 @@ public Analytics build() { if (threadFactory == null) { threadFactory = Platform.get().defaultThreadFactory(); } - if (callbacks == null) { - callbacks = Collections.emptyList(); - } else { - callbacks = Collections.unmodifiableList(callbacks); - } HttpLoggingInterceptor interceptor = new HttpLoggingInterceptor( @@ -463,7 +442,6 @@ public void log(String message) { log, threadFactory, networkExecutor, - callbacks, writeKey, gson); diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index 1a644aa5..c6cc9abd 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -59,7 +59,6 @@ public class AnalyticsClient { private final int maximumRetries; private final int maximumQueueByteSize; private final Log log; - private final List callbacks; private final ExecutorService networkExecutor; private final Thread looperThread; private final AtomicBoolean isShutDown; @@ -76,7 +75,6 @@ public static AnalyticsClient create( Log log, ThreadFactory threadFactory, ExecutorService networkExecutor, - List callbacks, String writeKey, Gson gsonInstance) { return new AnalyticsClient( @@ -91,7 +89,6 @@ public static AnalyticsClient create( log, threadFactory, networkExecutor, - callbacks, new AtomicBoolean(false), writeKey, gsonInstance); @@ -109,7 +106,6 @@ public AnalyticsClient( Log log, ThreadFactory threadFactory, ExecutorService networkExecutor, - List callbacks, AtomicBoolean isShutDown, String writeKey, Gson gsonInstance) { @@ -122,8 +118,7 @@ public AnalyticsClient( this.maximumRetries = maximumRetries; this.maximumQueueByteSize = maximumQueueSizeInBytes; this.log = log; - this.callbacks = callbacks; - this.looperThread = threadFactory.newThread(new Looper()); + this.looperThread = threadFactory.newThread(new Looper()); this.networkExecutor = networkExecutor; this.isShutDown = isShutDown; this.writeKey = writeKey; @@ -290,11 +285,7 @@ static BatchUploadTask create(AnalyticsClient client, Batch batch, int maxRetrie } private void notifyCallbacksWithException(Batch batch, Exception exception) { - for (Message message : batch.batch()) { - for (Callback callback : client.callbacks) { - callback.failure(message, exception); - } - } + // XXX failure } /** Returns {@code true} to indicate a batch should be retried. {@code false} otherwise. */ @@ -308,12 +299,7 @@ boolean upload() { if (response.isSuccessful()) { client.log.print(VERBOSE, "Uploaded batch %s.", batch.sequence()); - for (Message message : batch.batch()) { - for (Callback callback : client.callbacks) { - callback.success(message); - } - } - + // XXX success return false; } diff --git a/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java b/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java index 31596e90..e1c282ba 100644 --- a/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java +++ b/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java @@ -381,46 +381,12 @@ public void buildsWithThreadFactory() { assertThat(analytics).isNotNull(); } - @Test - public void nullCallback() { - try { - builder.callback(null); - fail("Should fail for null callback"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null callback"); - } - } - - @Test - public void duplicateCallback() { - Callback callback = mock(Callback.class); - try { - builder.callback(callback).callback(callback); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("Callback is already registered."); - } - } - - @Test - public void buildsWithValidCallback() { - Analytics analytics = builder.callback(mock(Callback.class)).build(); - assertThat(analytics).isNotNull(); - } - @Test public void buildsWithForceTlsV1() { Analytics analytics = builder.forceTlsVersion1().build(); assertThat(analytics).isNotNull(); } - @Test - public void multipleCallbacks() { - Analytics analytics = - builder.callback(mock(Callback.class)).callback(mock(Callback.class)).build(); - - assertThat(analytics).isNotNull(); - } - @Test public void nullPlugin() { try { diff --git a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java b/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java index b58ec35b..a06bd808 100644 --- a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java +++ b/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java @@ -4,20 +4,16 @@ import static com.segment.analytics.internal.StopMessage.STOP; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.openMocks; import com.google.gson.Gson; -import com.segment.analytics.Callback; import com.segment.analytics.Log; import com.segment.analytics.TestUtils.MessageBuilderTest; import com.segment.analytics.http.SegmentService; @@ -28,7 +24,6 @@ import com.segment.analytics.messages.TrackMessage; import com.segment.backo.Backo; import com.squareup.burst.BurstJUnit4; -import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -47,7 +42,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.Spy; import org.mockito.invocation.InvocationOnMock; @@ -78,7 +72,6 @@ public class AnalyticsClientTest { @Spy LinkedBlockingQueue pendingQueue; @Mock SegmentService segmentService; @Mock ExecutorService networkExecutor; - @Mock Callback callback; @Mock UploadResponse response; AtomicBoolean isShutDown; @@ -106,7 +99,6 @@ AnalyticsClient newClient() { log, threadFactory, networkExecutor, - Collections.singletonList(callback), isShutDown, writeKey, new Gson()); @@ -272,7 +264,6 @@ public void flushHowManyTimesNecessaryToStayWithinLimit() throws InterruptedExce log, threadFactory, networkExecutor, - Collections.singletonList(callback), isShutDown, writeKey, new Gson()); @@ -367,8 +358,8 @@ public void batchRetriesForNetworkErrors() { // Verify that we tried to upload 4 times, 3 failed and 1 succeeded. verify(segmentService, times(4)).upload(null, batch); - verify(callback).success(trackMessage); - } + //// verify(callback).success(trackMessage); + } @Test public void batchRetriesForHTTP5xxErrors() { @@ -392,8 +383,8 @@ public void batchRetriesForHTTP5xxErrors() { // Verify that we tried to upload 4 times, 3 failed and 1 succeeded. verify(segmentService, times(4)).upload(null, batch); - verify(callback).success(trackMessage); - } + // verify(callback).success(trackMessage); + } @Test public void batchRetriesForHTTP429Errors() { @@ -416,8 +407,8 @@ public void batchRetriesForHTTP429Errors() { // Verify that we tried to upload 4 times, 3 failed and 1 succeeded. verify(segmentService, times(4)).upload(null, batch); - verify(callback).success(trackMessage); - } + // verify(callback).success(trackMessage); + } @Test public void batchDoesNotRetryForNon5xxAndNon429HTTPErrors() { @@ -435,8 +426,8 @@ public void batchDoesNotRetryForNon5xxAndNon429HTTPErrors() { // Verify we only tried to upload once. verify(segmentService).upload(null, batch); - verify(callback).failure(eq(trackMessage), any(IOException.class)); - } + // verify(callback).failure(eq(trackMessage), any(IOException.class)); + } @Test public void batchDoesNotRetryForNonNetworkErrors() { @@ -452,8 +443,8 @@ public void batchDoesNotRetryForNonNetworkErrors() { // Verify we only tried to upload once. verify(segmentService).upload(null, batch); - verify(callback).failure(eq(trackMessage), any(RuntimeException.class)); - } + // verify(callback).failure(eq(trackMessage), any(RuntimeException.class)); + } @Test public void givesUpAfterMaxRetries() { @@ -477,17 +468,17 @@ public Call answer(InvocationOnMock invocation) { // DEFAULT_RETRIES == maxRetries // tries 11(one normal run + 10 retries) even though default is 50 in AnalyticsClient.java verify(segmentService, times(11)).upload(null, batch); - verify(callback) - .failure( - eq(trackMessage), - argThat( - new ArgumentMatcher() { - @Override - public boolean matches(IOException exception) { - return exception.getMessage().equals("11 retries exhausted"); - } - })); - } + // verify(callback) + // .failure( + // eq(trackMessage), + // argThat( + // new ArgumentMatcher() { + // @Override + // public boolean matches(IOException exception) { + // return exception.getMessage().equals("11 retries exhausted"); + // } + // })); + } @Test public void hasDefaultRetriesSetTo3() { @@ -511,17 +502,17 @@ public Call answer(InvocationOnMock invocation) { // DEFAULT_RETRIES == maxRetries // tries 11(one normal run + 10 retries) verify(segmentService, times(4)).upload(null, batch); - verify(callback) - .failure( - eq(trackMessage), - argThat( - new ArgumentMatcher() { - @Override - public boolean matches(IOException exception) { - return exception.getMessage().equals("4 retries exhausted"); - } - })); - } + // verify(callback) + // .failure( + // eq(trackMessage), + // argThat( + // new ArgumentMatcher() { + // @Override + // public boolean matches(IOException exception) { + // return exception.getMessage().equals("4 retries exhausted"); + // } + // })); + } @Test public void enqueueWithRegularMessageWhenNotShutdown(MessageBuilderTest builder) @@ -563,8 +554,8 @@ public void shutdownWhenAlreadyShutDown() throws InterruptedException { client.shutdown(); verify(messageQueue, times(0)).put(any(Message.class)); - verifyNoInteractions(networkExecutor, callback, segmentService); - } + // verifyNoInteractions(networkExecutor, callback, segmentService); + } @Test public void shutdownWithNoMessageInTheQueue() throws InterruptedException { @@ -612,17 +603,17 @@ public Call answer(InvocationOnMock invocation) { // runs once but never retries verify(segmentService, times(1)).upload(null, batch); - verify(callback) - .failure( - eq(trackMessage), - argThat( - new ArgumentMatcher() { - @Override - public boolean matches(IOException exception) { - return exception.getMessage().equals("1 retries exhausted"); - } - })); - } + // verify(callback) + // .failure( + // eq(trackMessage), + // argThat( + // new ArgumentMatcher() { + // @Override + // public boolean matches(IOException exception) { + // return exception.getMessage().equals("1 retries exhausted"); + // } + // })); + } /** * ********************************************************************************************** @@ -831,7 +822,6 @@ public void submitBatchBelowThreshold() throws InterruptedException, IllegalArgu log, threadFactory, networkExecutor, - Collections.singletonList(callback), isShutDown, writeKey, new Gson()); @@ -874,7 +864,6 @@ public void submitBatchAboveThreshold() throws InterruptedException, IllegalArgu log, threadFactory, networkExecutor, - Collections.singletonList(callback), isShutDown, writeKey, new Gson()); @@ -910,7 +899,6 @@ public void submitManySmallMessagesBatchAboveThreshold() throws InterruptedExcep log, threadFactory, networkExecutor, - Collections.singletonList(callback), isShutDown, writeKey, new Gson()); From af74b24ed91b875d15fa419f5ce56a2743cf50b2 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 24 Mar 2025 17:55:37 +0100 Subject: [PATCH 06/16] remove backo, add failsafe --- analytics/pom.xml | 12 +- .../java/com/segment/analytics/Analytics.java | 40 +--- .../analytics/internal/AnalyticsClient.java | 218 +++++++----------- .../com/segment/analytics/SegmentTest.java | 41 +++- 4 files changed, 139 insertions(+), 172 deletions(-) diff --git a/analytics/pom.xml b/analytics/pom.xml index 6a97d231..3f6be8ef 100644 --- a/analytics/pom.xml +++ b/analytics/pom.xml @@ -39,10 +39,18 @@ findbugs provided + - com.segment.backo - backo + dev.failsafe + failsafe + 3.3.2 + + dev.failsafe + failsafe-retrofit + 3.3.2 + + junit junit diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java index d7444643..bccce351 100644 --- a/analytics/src/main/java/com/segment/analytics/Analytics.java +++ b/analytics/src/main/java/com/segment/analytics/Analytics.java @@ -139,7 +139,6 @@ public static class Builder { private ThreadFactory threadFactory; private int flushQueueSize; private int maximumFlushAttempts; - private int maximumQueueSizeInBytes; private long flushIntervalInMillis; private int queueCapacity; private boolean forceTlsV1 = false; @@ -267,17 +266,6 @@ public Builder flushQueueSize(int flushQueueSize) { return this; } - /** Set the queueSize at which flushes should be triggered. */ - @Beta - public Builder maximumQueueSizeInBytes(int bytes) { - if (bytes < 1) { - throw new IllegalArgumentException("maximumQueueSizeInBytes must not be less than 1."); - } - - this.maximumQueueSizeInBytes = bytes; - return this; - } - /** Set the interval at which the queue should be flushed. */ @Beta public Builder flushInterval(long flushInterval, TimeUnit unit) { @@ -369,9 +357,6 @@ public Analytics build() { if (flushQueueSize == 0) { flushQueueSize = Platform.get().defaultFlushQueueSize(); } - if (maximumQueueSizeInBytes == 0) { - maximumQueueSizeInBytes = MESSAGE_QUEUE_MAX_BYTE_SIZE; - } if (maximumFlushAttempts == 0) { maximumFlushAttempts = 3; } @@ -430,20 +415,17 @@ public void log(String message) { SegmentService segmentService = restAdapter.create(SegmentService.class); - AnalyticsClient analyticsClient = - AnalyticsClient.create( - endpoint, - segmentService, - queueCapacity, - flushQueueSize, - flushIntervalInMillis, - maximumFlushAttempts, - maximumQueueSizeInBytes, - log, - threadFactory, - networkExecutor, - writeKey, - gson); + AnalyticsClient analyticsClient = AnalyticsClient.create( + endpoint, + segmentService, + queueCapacity, + flushQueueSize, + flushIntervalInMillis, + log, + threadFactory, + networkExecutor, + writeKey, + gson); return new Analytics(analyticsClient, messageTransformers, messageInterceptors, log); } diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index c6cc9abd..9f9e25b0 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -5,24 +5,30 @@ import static com.segment.analytics.Log.Level.VERBOSE; import com.google.gson.Gson; -import com.segment.analytics.Callback; import com.segment.analytics.Log; import com.segment.analytics.http.SegmentService; import com.segment.analytics.http.UploadResponse; import com.segment.analytics.messages.Batch; import com.segment.analytics.messages.Message; -import com.segment.backo.Backo; +import dev.failsafe.CircuitBreaker; +import dev.failsafe.CircuitBreakerOpenException; +import dev.failsafe.Failsafe; +import dev.failsafe.FailsafeExecutor; +import dev.failsafe.RetryPolicy; +import dev.failsafe.retrofit.FailsafeCall; import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; import java.util.LinkedList; -import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; @@ -38,7 +44,7 @@ public class AnalyticsClient { private static final int MSG_MAX_SIZE = 1024 * 32; private static final Charset ENCODING = StandardCharsets.UTF_8; private Gson gsonInstance; - private static final String instanceId = UUID.randomUUID().toString(); + private static final String instanceId = UUID.randomUUID().toString(); // TODO configurable ? static { Map library = new LinkedHashMap<>(); @@ -51,18 +57,16 @@ public class AnalyticsClient { } private final BlockingQueue messageQueue; - private final BlockingQueue pendingQueue; private final HttpUrl uploadUrl; private final SegmentService service; - private final int size; + private final int flushQueueSize; private final long flushIntervalInMillis; - private final int maximumRetries; - private final int maximumQueueByteSize; private final Log log; private final ExecutorService networkExecutor; private final Thread looperThread; private final AtomicBoolean isShutDown; private final String writeKey; + private final FailsafeExecutor> failsafe; public static AnalyticsClient create( HttpUrl uploadUrl, @@ -70,22 +74,17 @@ public static AnalyticsClient create( int queueCapacity, int flushQueueSize, long flushIntervalInMillis, - int maximumRetries, - int maximumQueueSizeInBytes, Log log, ThreadFactory threadFactory, ExecutorService networkExecutor, String writeKey, Gson gsonInstance) { return new AnalyticsClient( - new LinkedBlockingQueue(queueCapacity), new LinkedBlockingQueue(queueCapacity), uploadUrl, segmentService, flushQueueSize, flushIntervalInMillis, - maximumRetries, - maximumQueueSizeInBytes, log, threadFactory, networkExecutor, @@ -96,13 +95,10 @@ public static AnalyticsClient create( public AnalyticsClient( BlockingQueue messageQueue, - BlockingQueue pendingQueue, HttpUrl uploadUrl, SegmentService service, - int maxQueueSize, + int flushQueueSize, long flushIntervalInMillis, - int maximumRetries, - int maximumQueueSizeInBytes, Log log, ThreadFactory threadFactory, ExecutorService networkExecutor, @@ -110,13 +106,10 @@ public AnalyticsClient( String writeKey, Gson gsonInstance) { this.messageQueue = messageQueue; - this.pendingQueue = pendingQueue; this.uploadUrl = uploadUrl; this.service = service; - this.size = maxQueueSize; + this.flushQueueSize = flushQueueSize; this.flushIntervalInMillis = flushIntervalInMillis; - this.maximumRetries = maximumRetries; - this.maximumQueueByteSize = maximumQueueSizeInBytes; this.log = log; this.looperThread = threadFactory.newThread(new Looper()); this.networkExecutor = networkExecutor; @@ -124,6 +117,35 @@ public AnalyticsClient( this.writeKey = writeKey; this.gsonInstance = gsonInstance; looperThread.start(); + + CircuitBreaker> breaker = CircuitBreaker.>builder() + // 2 failure in 5 minute open the circuit + .withFailureThreshold(2, Duration.ofMinutes(5)) + // once open wait 1 minute to be half-open + .withDelay(Duration.ofMinutes(1)) + // after 1 success the circuit is closed + .withSuccessThreshold(1) + // 5xx or rate limit is an error + .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) + .build(); + + RetryPolicy> retry = RetryPolicy.>builder() + .withMaxAttempts(3) + .withBackoff(1, 300, ChronoUnit.SECONDS) + .withJitter(.1) + // retry on IOException + .handle(IOException.class) + // retry on 5xx or rate limit + .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) + .onRetriesExceeded(context -> { + throw new RuntimeException("retries"); + }) + .onAbort(context -> { + throw new RuntimeException("aborted"); + }) + .build(); + + this.failsafe = Failsafe.with(retry, breaker).with(networkExecutor); } public int messageSizeInBytes(Message message) { @@ -137,23 +159,14 @@ public boolean offer(Message message) { } public void enqueue(Message message) { - - enqueueSend(message); - } - - public void enqueueSend(Message message) { - if (isShutDown.get()) { - log.print(ERROR, "Attempt to enqueue a message when shutdown has been called %s.", message); - return; - } - - try { - messageQueue.put(message); - } catch (InterruptedException e) { - log.print(ERROR, e, "Interrupted while adding message %s.", message); - Thread.currentThread().interrupt(); + if (isShutDown.get()) { + log.print(ERROR, "Attempt to enqueue a message when shutdown has been called %s.", message); + return; + } + if (!messageQueue.offer(message)) { + handleError(message); + } } - } public void shutdown() { if (isShutDown.compareAndSet(false, true)) { @@ -171,6 +184,8 @@ public void shutdown() { public void shutdownAndWait(ExecutorService executor, String name) { try { + this.looperThread.interrupt(); + executor.shutdown(); final boolean executorTerminated = executor.awaitTermination(1, TimeUnit.SECONDS); @@ -228,7 +243,7 @@ public void run() { } Boolean isBlockingSignal = message == null; - Boolean isOverflow = messages.size() >= size; + Boolean isOverflow = messages.size() >= flushQueueSize; if (!messages.isEmpty() && (isOverflow || isBlockingSignal || batchSizeLimitReached)) { Batch batch = Batch.create(CONTEXT, new ArrayList<>(messages), writeKey); @@ -237,10 +252,22 @@ public void run() { "Batching %s message(s) into batch %s.", batch.batch().size(), batch.sequence()); - networkExecutor.submit( - BatchUploadTask.create(AnalyticsClient.this, batch, maximumRetries)); - currentBatchSize=0; + Call call = service.upload(uploadUrl, batch); + FailsafeCall failsafeCall = + FailsafeCall.with(failsafe).compose(call); + failsafeCall.executeAsync() + .thenAccept(r -> { + if(is5xx(r.code()) || r.code() == 429) { + handleError(batch, null); + } + }) + .exceptionally(t -> { + handleError(batch, t); + return null; + }); + + currentBatchSize = 0; messages.clear(); if (batchSizeLimitReached) { // If this is true that means the last message that would make us go over the limit @@ -253,109 +280,30 @@ public void run() { } } catch (InterruptedException e) { log.print(DEBUG, "Looper interrupted while polling for messages."); - Thread.currentThread().interrupt(); //XXX - } + // XXX CANCEL UPLOAD + } catch (Exception e) { + e.printStackTrace(); + } // SEND pending log.print(VERBOSE, "Looper stopped"); } + } - - static class BatchUploadTask implements Runnable { - private static final Backo BACKO = - Backo.builder() // - .base(TimeUnit.SECONDS, 15) // - .cap(TimeUnit.HOURS, 1) // - .jitter(1) // - .build(); - - private final AnalyticsClient client; - private final Backo backo; - final Batch batch; - private final int maxRetries; - - static BatchUploadTask create(AnalyticsClient client, Batch batch, int maxRetries) { - return new BatchUploadTask(client, BACKO, batch, maxRetries); - } - - BatchUploadTask(AnalyticsClient client, Backo backo, Batch batch, int maxRetries) { - this.client = client; - this.batch = batch; - this.backo = backo; - this.maxRetries = maxRetries; - } - - private void notifyCallbacksWithException(Batch batch, Exception exception) { - // XXX failure - } - - /** Returns {@code true} to indicate a batch should be retried. {@code false} otherwise. */ - boolean upload() { - client.log.print(VERBOSE, "Uploading batch %s.", batch.sequence()); - - try { - Call call = client.service.upload(client.uploadUrl, batch); - Response response = call.execute(); - - if (response.isSuccessful()) { - client.log.print(VERBOSE, "Uploaded batch %s.", batch.sequence()); - - // XXX success - return false; - } - - int status = response.code(); - if (is5xx(status)) { - client.log.print( - DEBUG, "Could not upload batch %s due to server error. Retrying.", batch.sequence()); - return true; - } else if (status == 429) { - client.log.print( - DEBUG, "Could not upload batch %s due to rate limiting. Retrying.", batch.sequence()); - return true; - } - - client.log.print(DEBUG, "Could not upload batch %s. Giving up.", batch.sequence()); - notifyCallbacksWithException(batch, new IOException(response.errorBody().string())); - - return false; - } catch (IOException error) { - client.log.print(DEBUG, error, "Could not upload batch %s. Retrying.", batch.sequence()); - - return true; - } catch (Exception exception) { - client.log.print(DEBUG, "Could not upload batch %s. Giving up.", batch.sequence()); - - notifyCallbacksWithException(batch, exception); - - return false; + + void handleError(Batch batch, Throwable t) { + if(t instanceof CompletionException && t.getCause() instanceof CircuitBreakerOpenException) { + System.err.println("OPEN"); } - } - - @Override - public void run() { - int attempt = 0; - for (; attempt <= maxRetries; attempt++) { - boolean retry = upload(); - if (!retry) return; - try { - backo.sleep(attempt); - } catch (InterruptedException e) { - client.log.print( - DEBUG, "Thread interrupted while backing off for batch %s.", batch.sequence()); - return; - } - } - - client.log.print(ERROR, "Could not upload batch %s. Retries exhausted.", batch.sequence()); - notifyCallbacksWithException( - batch, new IOException(Integer.toString(attempt) + " retries exhausted")); - } + + System.err.println("" + batch); + } + void handleError(Message message) { + System.err.println("" + message); + } private static boolean is5xx(int status) { return status >= 500 && status < 600; - } } - public static class BatchUtility { /** diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index 5af61174..864f9d51 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -6,6 +6,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.github.tomakehurst.wiremock.stubbing.ServeEvent; import com.google.gson.Gson; @@ -27,12 +28,17 @@ import wiremock.com.fasterxml.jackson.core.JsonProcessingException; import wiremock.com.fasterxml.jackson.databind.JsonNode; import wiremock.com.fasterxml.jackson.databind.ObjectMapper; +import wiremock.com.google.common.util.concurrent.RateLimiter; public class SegmentTest { @Rule - public WireMockRule wireMockRule = - new WireMockRule(wireMockConfig().dynamicPort().gzipDisabled(true), false); + public WireMockRule wireMockRule = new WireMockRule( + wireMockConfig() + .port(8088) + // .dynamicPort() + .gzipDisabled(true), + false); Analytics analytics; @@ -48,8 +54,10 @@ public void confWireMock() { analytics = Analytics.builder("write-key") .endpoint(wireMockRule.baseUrl()) + // .endpoint("http://localhost:8888") .flushInterval(1, TimeUnit.SECONDS) - .queueCapacity(500) + .flushQueueSize(20) + .queueCapacity(50) // callback // http client .build(); @@ -57,10 +65,31 @@ public void confWireMock() { @Test public void test() throws Throwable { - analytics.enqueue(TrackMessage.builder("my-track").messageId("m1").userId("userId")); - analytics.enqueue(TrackMessage.builder("my-track").messageId("m2").userId("userId")); - Awaitility.await().until(() -> sentMessagesEqualsTo("m1", "m2")); + stubFor(post(urlEqualTo("/v1/import/")) + .willReturn(WireMock.aResponse().withStatus(503).withBody("fail"))); + + long start = System.currentTimeMillis(); + boolean upAgain = false; + int id = 0; + RateLimiter rate = RateLimiter.create(5); + while (true) { + if (rate.tryAcquire()) { + System.err.println("id " + id); + analytics.enqueue( + TrackMessage.builder("my-track").messageId("m" + id++).userId("userId")); + } + Thread.sleep(50); + + if (!upAgain && System.currentTimeMillis() - start > 120_000) { + upAgain = true; + stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); + System.err.println("UP AGAIN"); + } + } + + // analytics.enqueue(TrackMessage.builder("my-track").messageId("m2").userId("userId")); + // Awaitility.await().until(() -> sentMessagesEqualsTo("m1", "m2")); } @Test From 85ef9096174d71eee6674c5bc8064d9485526a85 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Tue, 25 Mar 2025 13:08:37 +0100 Subject: [PATCH 07/16] fallback appender --- analytics/pom.xml | 1 - .../analytics/internal/AnalyticsClient.java | 38 +- .../analytics/internal/FallbackAppender.java | 227 +++++ .../analytics/internal/FlushMessage.java | 66 -- .../analytics/internal/StopMessage.java | 66 -- .../com/segment/analytics/AnalyticsTest.java | 144 --- .../com/segment/analytics/SegmentTest.java | 69 +- .../internal/AnalyticsClientTest.java | 921 ------------------ 8 files changed, 280 insertions(+), 1252 deletions(-) create mode 100644 analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java delete mode 100644 analytics/src/main/java/com/segment/analytics/internal/FlushMessage.java delete mode 100644 analytics/src/main/java/com/segment/analytics/internal/StopMessage.java delete mode 100644 analytics/src/test/java/com/segment/analytics/AnalyticsTest.java delete mode 100644 analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java diff --git a/analytics/pom.xml b/analytics/pom.xml index 3f6be8ef..4315a511 100644 --- a/analytics/pom.xml +++ b/analytics/pom.xml @@ -50,7 +50,6 @@ failsafe-retrofit 3.3.2 - junit junit diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index 9f9e25b0..124d399a 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -41,7 +41,6 @@ public class AnalyticsClient { private static final Map CONTEXT; private static final int BATCH_MAX_SIZE = 1024 * 500; - private static final int MSG_MAX_SIZE = 1024 * 32; private static final Charset ENCODING = StandardCharsets.UTF_8; private Gson gsonInstance; private static final String instanceId = UUID.randomUUID().toString(); // TODO configurable ? @@ -67,6 +66,7 @@ public class AnalyticsClient { private final AtomicBoolean isShutDown; private final String writeKey; private final FailsafeExecutor> failsafe; + private final FallbackAppender fallback; public static AnalyticsClient create( HttpUrl uploadUrl, @@ -119,10 +119,10 @@ public AnalyticsClient( looperThread.start(); CircuitBreaker> breaker = CircuitBreaker.>builder() - // 2 failure in 5 minute open the circuit - .withFailureThreshold(2, Duration.ofMinutes(5)) - // once open wait 1 minute to be half-open - .withDelay(Duration.ofMinutes(1)) + // 5 failure in 2 minute open the circuit + .withFailureThreshold(5, Duration.ofMinutes(2)) + // once open wait 30 seconds to be half-open + .withDelay(Duration.ofSeconds(30)) // after 1 success the circuit is closed .withSuccessThreshold(1) // 5xx or rate limit is an error @@ -130,9 +130,9 @@ public AnalyticsClient( .build(); RetryPolicy> retry = RetryPolicy.>builder() - .withMaxAttempts(3) + .withMaxAttempts(5) .withBackoff(1, 300, ChronoUnit.SECONDS) - .withJitter(.1) + .withJitter(.2) // retry on IOException .handle(IOException.class) // retry on 5xx or rate limit @@ -146,6 +146,7 @@ public AnalyticsClient( .build(); this.failsafe = Failsafe.with(retry, breaker).with(networkExecutor); + this.fallback = new FallbackAppender(this); } public int messageSizeInBytes(Message message) { @@ -167,13 +168,16 @@ public void enqueue(Message message) { handleError(message); } } + + // FIXME closeable public void shutdown() { if (isShutDown.compareAndSet(false, true)) { final long start = System.currentTimeMillis(); // first let's tell the system to stop looperThread.interrupt(); + fallback.close(); shutdownAndWait(networkExecutor, "network"); @@ -182,10 +186,8 @@ public void shutdown() { } } - public void shutdownAndWait(ExecutorService executor, String name) { + private void shutdownAndWait(ExecutorService executor, String name) { try { - this.looperThread.interrupt(); - executor.shutdown(); final boolean executorTerminated = executor.awaitTermination(1, TimeUnit.SECONDS); @@ -291,14 +293,18 @@ public void run() { } void handleError(Batch batch, Throwable t) { - if(t instanceof CompletionException && t.getCause() instanceof CircuitBreakerOpenException) { - System.err.println("OPEN"); + if(t instanceof CompletionException ) { + if(t.getCause() instanceof CircuitBreakerOpenException) { + System.err.println("OPEN"); + } + } + for(Message msg : batch.batch()) { + fallback.add(msg); } - - System.err.println("" + batch); } - void handleError(Message message) { - System.err.println("" + message); + + void handleError(Message msg) { + fallback.add(msg); } private static boolean is5xx(int status) { diff --git a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java new file mode 100644 index 00000000..b80bdab4 --- /dev/null +++ b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java @@ -0,0 +1,227 @@ +package com.segment.analytics.internal; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.segment.analytics.gson.AutoValueAdapterFactory; +import com.segment.analytics.gson.ISO8601DateAdapter; +import com.segment.analytics.messages.Message; +import com.segment.analytics.messages.TrackMessage; +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.channels.Channels; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; + +public class FallbackAppender { + + private static final int FLUSH_MS = 100; + private static final int BATCH = 20; + private static final int LASTMESSAGE_RETRY_MS = 10_000; + private static final String PATH = "pending"; + + private final AnalyticsClient client; + private final BlockingQueue queue; + private final File file; + private final Lock lock = new ReentrantLock(); + private final Thread writer; + private final Thread reader; + private final Gson gson; + + private transient long lastMessage; + + public FallbackAppender(AnalyticsClient client) { + this.client = client; + this.file = new File(PATH); + this.queue = new ArrayBlockingQueue(100); + this.writer = new Thread(new FileWriter()); // XXX threadFactory daemon + this.reader = new Thread(new FileReader()); // XXX threadFactory daemon + this.gson = new GsonBuilder() + .registerTypeAdapterFactory(new AutoValueAdapterFactory()) + .registerTypeAdapter(Date.class, new ISO8601DateAdapter()) + .create(); + + file.delete(); // FIXME do not remove on start + + this.lastMessage = System.currentTimeMillis(); + this.writer.start(); + this.reader.start(); + } + + public void close() { + reader.interrupt(); + writer.interrupt(); + } + + // block !!! + public void add(Message msg) { + try { + queue.put(msg); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + class FileReader implements Runnable { + @Override + public void run() { + while (!Thread.currentThread().isInterrupted()) { + if (queue.isEmpty() && System.currentTimeMillis() - lastMessage > LASTMESSAGE_RETRY_MS) { + if (file.length() == 0) { + continue; + } + + List msgs; + lock.lock(); + try { + msgs = read(); + if (msgs.isEmpty()) { + continue; + } + // FIXME now its reading all the msgs and waits until all is processed + // it will be better to work with batch and truncate the file + file.delete(); + } catch (IOException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + lastMessage = System.currentTimeMillis(); + continue; + } finally { + lock.unlock(); + } + + // FIXME batch + while (!msgs.isEmpty()) { + int reenqueued = 0; + boolean canEnqueue = true; + for (int i = msgs.size() - 1; canEnqueue && i >= 0; i--) { + Message msg = msgs.get(i); + canEnqueue = client.offer(msg); + if (canEnqueue) { + msgs.remove(i); + reenqueued++; + } + } + System.err.println("reenqueued " + reenqueued); + try { + Thread.sleep(1_000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + lastMessage = System.currentTimeMillis(); + } + + try { + Thread.sleep(1_000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + } + + class FileWriter implements Runnable { + @Override + public void run() { + final List batch = new ArrayList<>(); + while (!Thread.currentThread().isInterrupted()) { + try { + final Message msg = queue.poll(FLUSH_MS, TimeUnit.MILLISECONDS); + if (msg == null) { + if (!batch.isEmpty()) { + write(batch); + } + } else { + batch.add(msg); + if (batch.size() >= BATCH) { + write(batch); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + if (!batch.isEmpty()) { + write(batch); + } + } + } + + List read() throws IOException { + if (file.exists()) { + try (FileChannel fileChannel = FileChannel.open(file.toPath(), StandardOpenOption.READ)) { + fileChannel.lock(0, Long.MAX_VALUE, true); + + final String[] lines = new String( + Channels.newInputStream(fileChannel).readAllBytes(), StandardCharsets.UTF_8) + .split(System.lineSeparator()); + return Arrays.stream(lines) + .map(m -> fromJson(m)) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + } + } else { + return Collections.emptyList(); + } + } + + private void write(List batch) { + lock.lock(); + try (FileChannel fileChannel = FileChannel.open( + file.toPath(), StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE)) { + fileChannel.lock(); + + final String lines = batch.stream() + .map(this::toJson) + .filter(Objects::nonNull) + .collect(Collectors.joining(System.lineSeparator())); + + OutputStream os = Channels.newOutputStream(fileChannel); + os.write(lines.getBytes(StandardCharsets.UTF_8)); + os.write(System.lineSeparator().getBytes(StandardCharsets.UTF_8)); + fileChannel.force(true); + + batch.clear(); + + lastMessage = System.currentTimeMillis(); + } catch (IOException e) { + e.printStackTrace(); // FIXME + } finally { + lock.unlock(); + } + } + + private String toJson(final Message msg) { + try { + return gson.toJson(msg); + } catch (Exception e) { + e.printStackTrace(); + return null; + } + } + + private Message fromJson(final String msg) { + try { + // FIXME only track + return gson.fromJson(msg, TrackMessage.class); + } catch (Exception e) { + e.printStackTrace(); + return null; + } + } +} diff --git a/analytics/src/main/java/com/segment/analytics/internal/FlushMessage.java b/analytics/src/main/java/com/segment/analytics/internal/FlushMessage.java deleted file mode 100644 index b3ee9dc2..00000000 --- a/analytics/src/main/java/com/segment/analytics/internal/FlushMessage.java +++ /dev/null @@ -1,66 +0,0 @@ -package com.segment.analytics.internal; - -import com.segment.analytics.messages.Message; -import java.util.Date; -import java.util.Map; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -class FlushMessage implements Message { - static final FlushMessage POISON = new FlushMessage(); - - private FlushMessage() {} - - @Nonnull - @Override - public Type type() { - throw new UnsupportedOperationException(); - } - - @Nonnull - @Override - public String messageId() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public Date sentAt() { - throw new UnsupportedOperationException(); - } - - @Nonnull - @Override - public Date timestamp() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public Map context() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public String anonymousId() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public String userId() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public Map integrations() { - throw new UnsupportedOperationException(); - } - - @Override - public String toString() { - return "FlushMessage{}"; - } -} diff --git a/analytics/src/main/java/com/segment/analytics/internal/StopMessage.java b/analytics/src/main/java/com/segment/analytics/internal/StopMessage.java deleted file mode 100644 index eccd278c..00000000 --- a/analytics/src/main/java/com/segment/analytics/internal/StopMessage.java +++ /dev/null @@ -1,66 +0,0 @@ -package com.segment.analytics.internal; - -import com.segment.analytics.messages.Message; -import java.util.Date; -import java.util.Map; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -class StopMessage implements Message { - static final StopMessage STOP = new StopMessage(); - - private StopMessage() {} - - @Nonnull - @Override - public Type type() { - throw new UnsupportedOperationException(); - } - - @Nonnull - @Override - public String messageId() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public Date sentAt() { - throw new UnsupportedOperationException(); - } - - @Nonnull - @Override - public Date timestamp() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public Map context() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public String anonymousId() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public String userId() { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public Map integrations() { - throw new UnsupportedOperationException(); - } - - @Override - public String toString() { - return "StopMessage{}"; - } -} diff --git a/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java b/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java deleted file mode 100644 index 075ae003..00000000 --- a/analytics/src/test/java/com/segment/analytics/AnalyticsTest.java +++ /dev/null @@ -1,144 +0,0 @@ -package com.segment.analytics; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -import static org.mockito.MockitoAnnotations.initMocks; - -import com.segment.analytics.TestUtils.MessageBuilderTest; -import com.segment.analytics.internal.AnalyticsClient; -import com.segment.analytics.messages.Message; -import com.segment.analytics.messages.MessageBuilder; -import com.squareup.burst.BurstJUnit4; -import java.lang.reflect.Field; -import java.time.LocalDateTime; -import java.time.temporal.ChronoUnit; -import java.util.Collections; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; - -@RunWith(BurstJUnit4.class) -public class AnalyticsTest { - @Mock AnalyticsClient client; - @Mock Log log; - @Mock MessageTransformer messageTransformer; - @Mock MessageInterceptor messageInterceptor; - Analytics analytics; - - @Before - public void setUp() { - initMocks(this); - - analytics = - new Analytics( - client, - Collections.singletonList(messageTransformer), - Collections.singletonList(messageInterceptor), - log); - } - - @Test - public void enqueueIsDispatched(MessageBuilderTest builder) { - MessageBuilder messageBuilder = builder.get().userId("prateek"); - Message message = messageBuilder.build(); - when(messageTransformer.transform(messageBuilder)).thenReturn(true); - when(messageInterceptor.intercept(any(Message.class))).thenReturn(message); - - analytics.enqueue(messageBuilder); - - verify(messageTransformer).transform(messageBuilder); - verify(messageInterceptor).intercept(any(Message.class)); - verify(client).enqueue(message); - } - - @Test - public void doesNotEnqueueWhenTransformerReturnsFalse(MessageBuilderTest builder) { - MessageBuilder messageBuilder = builder.get().userId("prateek"); - when(messageTransformer.transform(messageBuilder)).thenReturn(false); - - analytics.enqueue(messageBuilder); - - verify(messageTransformer).transform(messageBuilder); - verify(messageInterceptor, never()).intercept(any(Message.class)); - verify(client, never()).enqueue(any(Message.class)); - } - - @Test - public void doesNotEnqueueWhenInterceptorReturnsNull(MessageBuilderTest builder) { - MessageBuilder messageBuilder = builder.get().userId("prateek"); - when(messageTransformer.transform(messageBuilder)).thenReturn(true); - - analytics.enqueue(messageBuilder); - - verify(messageTransformer).transform(messageBuilder); - verify(messageInterceptor).intercept(any(Message.class)); - verify(client, never()).enqueue(any(Message.class)); - } - - @Test - public void shutdownIsDispatched() { - analytics.shutdown(); - - verify(client).shutdown(); - } - - @Test - public void offerIsDispatched(MessageBuilderTest builder) { - MessageBuilder messageBuilder = builder.get().userId("dummy"); - Message message = messageBuilder.build(); - when(messageTransformer.transform(messageBuilder)).thenReturn(true); - when(messageInterceptor.intercept(any(Message.class))).thenReturn(message); - - analytics.offer(messageBuilder); - - verify(messageTransformer).transform(messageBuilder); - verify(messageInterceptor).intercept(any(Message.class)); - verify(client).offer(message); - } - - @Test - public void threadSafeTest(MessageBuilderTest builder) - throws NoSuchFieldException, IllegalAccessException, InterruptedException { - // we want to test if msgs get lost during a multithreaded env - Analytics analytics = Analytics.builder("testWriteKeyForIssue321").build(); - // So we just want to spy on the client of an Analytics object created normally - Field clientField = analytics.getClass().getDeclaredField("client"); - clientField.setAccessible(true); - AnalyticsClient spy = spy((AnalyticsClient) clientField.get(analytics)); - clientField.set(analytics, spy); - - // we are going to run this test for a specific amount of seconds - int millisRunning = 200; - LocalDateTime initialTime = LocalDateTime.now(); - LocalDateTime now; - - // and a set number of threads will be using the library - ExecutorService service = Executors.newFixedThreadPool(20); - AtomicInteger counter = new AtomicInteger(); - - MessageBuilder messageBuilder = builder.get().userId("jorgen25"); - - do { - service.submit( - () -> { - analytics.enqueue(messageBuilder); - counter.incrementAndGet(); - }); - now = LocalDateTime.now(); - } while (initialTime.until(now, ChronoUnit.MILLIS) < millisRunning); - - service.shutdown(); - service.awaitTermination(5, TimeUnit.SECONDS); - - verify(spy, times(counter.get())).enqueue(any(Message.class)); - } -} diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index 864f9d51..be00286a 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -9,19 +9,16 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.github.tomakehurst.wiremock.stubbing.ServeEvent; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; -import com.segment.analytics.gson.AutoValueAdapterFactory; -import com.segment.analytics.gson.ISO8601DateAdapter; import com.segment.analytics.messages.TrackMessage; import java.util.ArrayList; import java.util.Arrays; -import java.util.Date; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; import org.awaitility.Awaitility; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -42,68 +39,59 @@ public class SegmentTest { Analytics analytics; - GsonBuilder gsonBuilder = new GsonBuilder() - .registerTypeAdapterFactory(new AutoValueAdapterFactory()) - .registerTypeAdapter(Date.class, new ISO8601DateAdapter()); - - Gson gson = gsonBuilder.create(); - @Before - public void confWireMock() { + public void confWireMockAndClient() { stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); analytics = Analytics.builder("write-key") .endpoint(wireMockRule.baseUrl()) - // .endpoint("http://localhost:8888") .flushInterval(1, TimeUnit.SECONDS) .flushQueueSize(20) .queueCapacity(50) - // callback // http client .build(); } + @After + public void tearDown() { + analytics.shutdown(); + } + @Test public void test() throws Throwable { stubFor(post(urlEqualTo("/v1/import/")) - .willReturn(WireMock.aResponse().withStatus(503).withBody("fail"))); + .willReturn( + WireMock.aResponse().withStatus(503).withBody("fail").withUniformRandomDelay(100, 1_000))); long start = System.currentTimeMillis(); boolean upAgain = false; int id = 0; + List ids = new ArrayList<>(); RateLimiter rate = RateLimiter.create(5); - while (true) { + while (System.currentTimeMillis() - start < 60_000) { if (rate.tryAcquire()) { - System.err.println("id " + id); + String msgid = "m" + id++; + ids.add(msgid); analytics.enqueue( - TrackMessage.builder("my-track").messageId("m" + id++).userId("userId")); + TrackMessage.builder("my-track").messageId(msgid).userId("userId")); + System.err.println("enqued " + msgid); } + Thread.sleep(50); - if (!upAgain && System.currentTimeMillis() - start > 120_000) { + if (!upAgain && System.currentTimeMillis() - start > 20_000) { upAgain = true; - stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); + stubFor(post(urlEqualTo("/v1/import/")) + .willReturn(okJson("{\"success\": \"true\"}").withUniformRandomDelay(100, 1_000))); System.err.println("UP AGAIN"); } } - // analytics.enqueue(TrackMessage.builder("my-track").messageId("m2").userId("userId")); - // Awaitility.await().until(() -> sentMessagesEqualsTo("m1", "m2")); - } - - @Test - public void testMore() throws Throwable { - System.err.println("wm at " + wireMockRule.baseUrl()); - int num = 100_000; - String[] expectedIds = new String[num]; - for (int i = 0; i < num; i++) { - String id = "m" + i; - expectedIds[i] = id; - analytics.enqueue(TrackMessage.builder("my-track").messageId(id).userId("userId")); - } - - Awaitility.await().atMost(1, TimeUnit.MINUTES).until(() -> sentMessagesEqualsTo(expectedIds)); + Awaitility.await() + .atMost(10, TimeUnit.MINUTES) + .pollInterval(1, TimeUnit.SECONDS) + .until(() -> sentMessagesEqualsTo(ids.toArray(new String[ids.size()]))); } private static final ObjectMapper OM = new ObjectMapper(); @@ -112,9 +100,13 @@ private boolean sentMessagesEqualsTo(String... msgIds) { return new HashSet<>(sentMessages()).equals(new HashSet<>(Arrays.asList(msgIds))); } - private List sentMessages() { - List messageIds = new ArrayList<>(); + private Set sentMessages() { + Set messageIds = new HashSet<>(); for (ServeEvent event : wireMockRule.getAllServeEvents()) { + if (event.getResponse().getStatus() != 200) { + continue; + } + JsonNode batch; try { JsonNode json = OM.readTree(event.getRequest().getBodyAsString()); @@ -130,6 +122,7 @@ private List sentMessages() { messageIds.add(msgs.next().get("messageId").asText()); } } + System.err.println("Confirmed msgs : " + messageIds.size()); return messageIds; } } diff --git a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java b/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java deleted file mode 100644 index a06bd808..00000000 --- a/analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java +++ /dev/null @@ -1,921 +0,0 @@ -package com.segment.analytics.internal; - -import static com.segment.analytics.internal.FlushMessage.POISON; -import static com.segment.analytics.internal.StopMessage.STOP; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.timeout; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; -import static org.mockito.MockitoAnnotations.openMocks; - -import com.google.gson.Gson; -import com.segment.analytics.Log; -import com.segment.analytics.TestUtils.MessageBuilderTest; -import com.segment.analytics.http.SegmentService; -import com.segment.analytics.http.UploadResponse; -import com.segment.analytics.internal.AnalyticsClient.BatchUploadTask; -import com.segment.analytics.messages.Batch; -import com.segment.analytics.messages.Message; -import com.segment.analytics.messages.TrackMessage; -import com.segment.backo.Backo; -import com.squareup.burst.BurstJUnit4; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Queue; -import java.util.Random; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import okhttp3.ResponseBody; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.Spy; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; -import retrofit2.Call; -import retrofit2.Response; -import retrofit2.mock.Calls; - -@RunWith(BurstJUnit4.class) // -public class AnalyticsClientTest { - // Backo instance for testing which trims down the wait times. - private static final Backo BACKO = - Backo.builder().base(TimeUnit.NANOSECONDS, 1).factor(1).build(); - - private int DEFAULT_RETRIES = 10; - private int MAX_BATCH_SIZE = 1024 * 500; // 500kb - private int MAX_MSG_SIZE = 1024 * 32; // 32kb //This is the limit for a message object - private int MSG_MAX_CREATE_SIZE = - MAX_MSG_SIZE - - 200; // Once we create msg object with this size it barely below 32 threshold so good - // for tests - private static String writeKey = "writeKey"; - - Log log = Log.NONE; - - ThreadFactory threadFactory; - @Spy LinkedBlockingQueue messageQueue; - @Spy LinkedBlockingQueue pendingQueue; - @Mock SegmentService segmentService; - @Mock ExecutorService networkExecutor; - @Mock UploadResponse response; - - AtomicBoolean isShutDown; - - @Before - public void setUp() { - openMocks(this); - - isShutDown = new AtomicBoolean(false); - threadFactory = Executors.defaultThreadFactory(); - } - - // Defers loading the client until tests can initialize all required - // dependencies. - AnalyticsClient newClient() { - return new AnalyticsClient( - messageQueue, - pendingQueue, - null, - segmentService, - 50, - TimeUnit.HOURS.toMillis(1), - 0, - MAX_BATCH_SIZE, - log, - threadFactory, - networkExecutor, - isShutDown, - writeKey, - new Gson()); - } - - @Test - public void enqueueAddsToQueue(MessageBuilderTest builder) throws InterruptedException { - AnalyticsClient client = newClient(); - - Message message = builder.get().userId("prateek").build(); - client.enqueue(message); - - verify(messageQueue).put(message); - } - - @Test - public void shutdown() throws InterruptedException { - messageQueue = new LinkedBlockingQueue<>(); - AnalyticsClient client = newClient(); - - client.shutdown(); - - verify(networkExecutor).shutdown(); - verify(networkExecutor).awaitTermination(1, TimeUnit.SECONDS); - } - - /** Wait until the queue is drained. */ - static void wait(Queue queue) { - // noinspection StatementWithEmptyBody - while (queue.size() > 0) {} - } - - /** - * Verify that a {@link BatchUploadTask} was submitted to the executor, and return the {@link - * BatchUploadTask#batch} it was uploading.. - */ - static Batch captureBatch(ExecutorService executor) { - final ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); - verify(executor, timeout(1000)).submit(runnableArgumentCaptor.capture()); - final BatchUploadTask task = (BatchUploadTask) runnableArgumentCaptor.getValue(); - return task.batch; - } - - private static String generateDataOfSize(int msgSize) { - char[] chars = new char[msgSize]; - Arrays.fill(chars, 'a'); - - return new String(chars); - } - - private static String generateDataOfSizeSpecialChars( - int sizeInBytes, boolean slightlyBelowLimit) { - StringBuilder builder = new StringBuilder(); - Character[] specialChars = new Character[] {'$', '¢', 'ह', '€', '한', '©', '¶'}; - int currentSize = 0; - String smileyFace = "\uD83D\uDE01"; - // 😁 = '\uD83D\uDE01'; - Random rand = new Random(); - int loopCount = 1; - while (currentSize < sizeInBytes) { - int randomNum; - // decide if regular/special character - if (loopCount > 3 && loopCount % 4 == 0) { - randomNum = rand.nextInt(((specialChars.length - 1) - 0) + 1) + 0; - builder.append(specialChars[randomNum]); - } else if (loopCount > 9 && loopCount % 10 == 0) { - builder.append(smileyFace); - } else { - // random letter from a - z - randomNum = rand.nextInt(('z' - 'a') + 1) + 'a'; - builder.append((char) randomNum); - } - - // check size so far - String temp = builder.toString(); - currentSize = temp.getBytes(StandardCharsets.UTF_8).length; - if (slightlyBelowLimit && ((sizeInBytes - currentSize) < 500)) { - break; - } - loopCount++; - } - return builder.toString(); - } - - @Test - public void enqueueMaxTriggersFlush() { - messageQueue = new LinkedBlockingQueue<>(); - AnalyticsClient client = newClient(); - - // Enqueuing 51 messages (> 50) should trigger flush. - for (int i = 0; i < 51; i++) { - client.enqueue(TrackMessage.builder("Event " + i).userId("bar").build()); - } - wait(messageQueue); - - // Verify that the executor saw the batch. - assertThat(captureBatch(networkExecutor).batch()).hasSize(50); - } - - @Test - public void shouldBeAbleToCalculateMessageSize() { - AnalyticsClient client = newClient(); - Map properties = new HashMap(); - - properties.put("property1", generateDataOfSize(1024 * 33)); - - TrackMessage bigMessage = - TrackMessage.builder("Big Event").userId("bar").properties(properties).build(); - try { - client.enqueue(bigMessage); - } catch (IllegalArgumentException e) { - assertThat(e).isExactlyInstanceOf(e.getClass()); - } - - // can't test for exact size cause other attributes come in play - assertThat(client.messageSizeInBytes(bigMessage)).isGreaterThan(1024 * 33); - } - - @Test - public void dontFlushUntilReachesMaxSize() throws InterruptedException { - AnalyticsClient client = newClient(); - Map properties = new HashMap(); - - properties.put("property2", generateDataOfSize(MAX_BATCH_SIZE - 200)); - - TrackMessage bigMessage = - TrackMessage.builder("Big Event").userId("bar").properties(properties).build(); - try { - client.enqueue(bigMessage); - } catch (IllegalArgumentException e) { - // throw new InterruptedException(e.getMessage()); - } - - wait(messageQueue); - - verify(networkExecutor, never()).submit(any(Runnable.class)); - } - - /** - * Modified this test case since we are changing logic to NOT allow messages bigger than 32 kbs - * individually to be enqueued, hence had to lower the size of the generated msg here. chose - * MSG_MAX_CREATE_SIZE because it will generate a message just below the limit of 32 kb after it - * creates a Message object modified the number of events that will be created since the batch - * creation logic was also changed to not allow batches larger than 500 kb meaning every 15/16 - * events the queue will be backPressured and poisoned/flushed (3 times) (purpose of test) AND - * there will be 4 batches submitted (15 msgs, 1 msg, 15 msg, 15 msg) so purpose of test case - * stands - * - * @throws InterruptedException - */ - @Test - public void flushHowManyTimesNecessaryToStayWithinLimit() throws InterruptedException { - AnalyticsClient client = - new AnalyticsClient( - messageQueue, - pendingQueue, - null, - segmentService, - 50, - TimeUnit.HOURS.toMillis(1), - 0, - MAX_BATCH_SIZE * 4, - log, - threadFactory, - networkExecutor, - isShutDown, - writeKey, - new Gson()); - - Map properties = new HashMap(); - - properties.put("property3", generateDataOfSize(MSG_MAX_CREATE_SIZE)); - - for (int i = 0; i < 46; i++) { - TrackMessage bigMessage = - TrackMessage.builder("Big Event").userId("bar").properties(properties).build(); - client.enqueue(bigMessage); - verify(messageQueue).put(bigMessage); - } - - wait(messageQueue); - /** - * modified from expected 4 to expected 3 times, since we removed the inner loop. The inner loop - * was forcing to message list created from the queue to keep making batches even if its a 1 - * message batch until the message list is empty, that was forcing the code to make one last - * batch of 1 msg in size bumping the number of times a batch would be submitted from 3 to 4 - */ - verify(networkExecutor, times(3)).submit(any(Runnable.class)); - } - - /** - * Had to slightly change test case since we are now modifying the logic to NOT allow messages - * above 32 KB in size So needed to change size of generated msg to MSG_MAX_CREATE_SIZE to keep - * purpose of test case intact which is to test the scenario for several messages eventually - * filling up the queue and flushing. Batches submitted will change from 1 to 2 because the queue - * will be backpressured at 16 (at this point queue is over the 500KB batch limit so its flushed - * and when batch is created 16 will be above 500kbs limit so it creates one batch for 15 msg and - * another one for the remaining single message so 500kb limit per batch is not violated - * - * @throws InterruptedException - */ - @Test - public void flushWhenMultipleMessagesReachesMaxSize() throws InterruptedException { - AnalyticsClient client = newClient(); - Map properties = new HashMap(); - properties.put("property3", generateDataOfSize(MSG_MAX_CREATE_SIZE)); - - for (int i = 0; i < 16; i++) { - TrackMessage bigMessage = - TrackMessage.builder("Big Event").userId("bar").properties(properties).build(); - client.enqueue(bigMessage); - } - wait(messageQueue); - client.shutdown(); - while (!isShutDown.get()) {} - verify(networkExecutor, times(2)).submit(any(Runnable.class)); - } - - @Test - public void enqueueBeforeMaxDoesNotTriggerFlush() { - messageQueue = new LinkedBlockingQueue<>(); - AnalyticsClient client = newClient(); - - // Enqueuing 5 messages (< 50) should not trigger flush. - for (int i = 0; i < 5; i++) { - client.enqueue(TrackMessage.builder("Event " + i).userId("bar").build()); - } - wait(messageQueue); - - // Verify that the executor didn't see anything. - verify(networkExecutor, never()).submit(any(Runnable.class)); - } - - static Batch batchFor(Message message) { - return Batch.create( - Collections.emptyMap(), Collections.singletonList(message), writeKey); - } - - @Test - public void batchRetriesForNetworkErrors() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - Response successResponse = Response.success(200, response); - Response failureResponse = Response.error(429, ResponseBody.create(null, "")); - - // Throw a network error 3 times. - when(segmentService.upload(null, batch)) - .thenReturn(Calls.response(failureResponse)) - .thenReturn(Calls.response(failureResponse)) - .thenReturn(Calls.response(failureResponse)) - .thenReturn(Calls.response(successResponse)); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES); - batchUploadTask.run(); - - // Verify that we tried to upload 4 times, 3 failed and 1 succeeded. - verify(segmentService, times(4)).upload(null, batch); - //// verify(callback).success(trackMessage); - } - - @Test - public void batchRetriesForHTTP5xxErrors() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - // Throw a HTTP error 3 times. - - Response successResponse = Response.success(200, response); - Response failResponse = - Response.error(500, ResponseBody.create(null, "Server Error")); - when(segmentService.upload(null, batch)) - .thenReturn(Calls.response(failResponse)) - .thenReturn(Calls.response(failResponse)) - .thenReturn(Calls.response(failResponse)) - .thenReturn(Calls.response(successResponse)); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES); - batchUploadTask.run(); - - // Verify that we tried to upload 4 times, 3 failed and 1 succeeded. - verify(segmentService, times(4)).upload(null, batch); - // verify(callback).success(trackMessage); - } - - @Test - public void batchRetriesForHTTP429Errors() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - // Throw a HTTP error 3 times. - Response successResponse = Response.success(200, response); - Response failResponse = - Response.error(429, ResponseBody.create(null, "Rate Limited")); - when(segmentService.upload(null, batch)) - .thenReturn(Calls.response(failResponse)) - .thenReturn(Calls.response(failResponse)) - .thenReturn(Calls.response(failResponse)) - .thenReturn(Calls.response(successResponse)); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES); - batchUploadTask.run(); - - // Verify that we tried to upload 4 times, 3 failed and 1 succeeded. - verify(segmentService, times(4)).upload(null, batch); - // verify(callback).success(trackMessage); - } - - @Test - public void batchDoesNotRetryForNon5xxAndNon429HTTPErrors() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - // Throw a HTTP error that should not be retried. - Response failResponse = - Response.error(404, ResponseBody.create(null, "Not Found")); - when(segmentService.upload(null, batch)).thenReturn(Calls.response(failResponse)); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES); - batchUploadTask.run(); - - // Verify we only tried to upload once. - verify(segmentService).upload(null, batch); - // verify(callback).failure(eq(trackMessage), any(IOException.class)); - } - - @Test - public void batchDoesNotRetryForNonNetworkErrors() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - Call networkFailure = Calls.failure(new RuntimeException()); - when(segmentService.upload(null, batch)).thenReturn(networkFailure); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES); - batchUploadTask.run(); - - // Verify we only tried to upload once. - verify(segmentService).upload(null, batch); - // verify(callback).failure(eq(trackMessage), any(RuntimeException.class)); - } - - @Test - public void givesUpAfterMaxRetries() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - when(segmentService.upload(null, batch)) - .thenAnswer( - new Answer>() { - public Call answer(InvocationOnMock invocation) { - Response failResponse = - Response.error(429, ResponseBody.create(null, "Not Found")); - return Calls.response(failResponse); - } - }); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, 10); - batchUploadTask.run(); - - // DEFAULT_RETRIES == maxRetries - // tries 11(one normal run + 10 retries) even though default is 50 in AnalyticsClient.java - verify(segmentService, times(11)).upload(null, batch); - // verify(callback) - // .failure( - // eq(trackMessage), - // argThat( - // new ArgumentMatcher() { - // @Override - // public boolean matches(IOException exception) { - // return exception.getMessage().equals("11 retries exhausted"); - // } - // })); - } - - @Test - public void hasDefaultRetriesSetTo3() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - when(segmentService.upload(null, batch)) - .thenAnswer( - new Answer>() { - public Call answer(InvocationOnMock invocation) { - Response failResponse = - Response.error(429, ResponseBody.create(null, "Not Found")); - return Calls.response(failResponse); - } - }); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, 3); - batchUploadTask.run(); - - // DEFAULT_RETRIES == maxRetries - // tries 11(one normal run + 10 retries) - verify(segmentService, times(4)).upload(null, batch); - // verify(callback) - // .failure( - // eq(trackMessage), - // argThat( - // new ArgumentMatcher() { - // @Override - // public boolean matches(IOException exception) { - // return exception.getMessage().equals("4 retries exhausted"); - // } - // })); - } - - @Test - public void enqueueWithRegularMessageWhenNotShutdown(MessageBuilderTest builder) - throws InterruptedException { - AnalyticsClient client = newClient(); - - final Message message = builder.get().userId("foo").build(); - client.enqueue(message); - - verify(messageQueue).put(message); - } - - @Test - public void enqueueWithRegularMessageWhenShutdown(MessageBuilderTest builder) - throws InterruptedException { - AnalyticsClient client = newClient(); - isShutDown.set(true); - - client.enqueue(builder.get().userId("foo").build()); - - verify(messageQueue, times(0)).put(any(Message.class)); - } - - @Test - public void enqueueWithStopMessageWhenShutdown() throws InterruptedException { - AnalyticsClient client = newClient(); - isShutDown.set(true); - - client.enqueue(STOP); - - verify(messageQueue).put(STOP); - } - - @Test - public void shutdownWhenAlreadyShutDown() throws InterruptedException { - AnalyticsClient client = newClient(); - isShutDown.set(true); - - client.shutdown(); - - verify(messageQueue, times(0)).put(any(Message.class)); - // verifyNoInteractions(networkExecutor, callback, segmentService); - } - - @Test - public void shutdownWithNoMessageInTheQueue() throws InterruptedException { - AnalyticsClient client = newClient(); - client.shutdown(); - - verify(messageQueue).put(STOP); - verify(networkExecutor).shutdown(); - verify(networkExecutor).awaitTermination(1, TimeUnit.SECONDS); - verifyNoMoreInteractions(networkExecutor); - } - - @Test - public void shutdownWithMessagesInTheQueue(MessageBuilderTest builder) - throws InterruptedException { - AnalyticsClient client = newClient(); - - client.enqueue(builder.get().userId("foo").build()); - client.shutdown(); - - verify(messageQueue).put(STOP); - verify(networkExecutor).shutdown(); - verify(networkExecutor).awaitTermination(1, TimeUnit.SECONDS); - verify(networkExecutor).submit(any(AnalyticsClient.BatchUploadTask.class)); - } - - @Test - public void neverRetries() { - AnalyticsClient client = newClient(); - TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build(); - Batch batch = batchFor(trackMessage); - - when(segmentService.upload(null, batch)) - .thenAnswer( - new Answer>() { - public Call answer(InvocationOnMock invocation) { - Response failResponse = - Response.error(429, ResponseBody.create(null, "Not Found")); - return Calls.response(failResponse); - } - }); - - BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, 0); - batchUploadTask.run(); - - // runs once but never retries - verify(segmentService, times(1)).upload(null, batch); - // verify(callback) - // .failure( - // eq(trackMessage), - // argThat( - // new ArgumentMatcher() { - // @Override - // public boolean matches(IOException exception) { - // return exception.getMessage().equals("1 retries exhausted"); - // } - // })); - } - - /** - * ********************************************************************************************** - * Test cases for Size check - * ********************************************************************************************* - */ - - /** Individual Size check happy path regular chars */ - @Test - public void checkForIndividualMessageSizeLessThanLimit() { - AnalyticsClient client = newClient(); - int msgSize = 1024 * 31; // 31KB - int sizeLimit = MAX_MSG_SIZE; // 32KB = 32768 - Map properties = new HashMap(); - - properties.put("property1", generateDataOfSize(msgSize)); - - TrackMessage bigMessage = - TrackMessage.builder("Event").userId("jorgen25").properties(properties).build(); - client.enqueue(bigMessage); - - int msgActualSize = client.messageSizeInBytes(bigMessage); - assertThat(msgActualSize).isLessThanOrEqualTo(sizeLimit); - } - - /** Individual Size check sad path regular chars (over the limit) */ - @Test - public void checkForIndividualMessageSizeOverLimit() throws IllegalArgumentException { - AnalyticsClient client = newClient(); - int msgSize = MAX_MSG_SIZE + 1; // BARELY over the limit - int sizeLimit = MAX_MSG_SIZE; // 32KB = 32768 - Map properties = new HashMap(); - - properties.put("property1", generateDataOfSize(msgSize)); - - TrackMessage bigMessage = - TrackMessage.builder("Event").userId("jorgen25").properties(properties).build(); - try { - client.enqueue(bigMessage); - } catch (IllegalArgumentException e) { - assertThat(e).isExactlyInstanceOf(e.getClass()); - } - - int msgActualSize = client.messageSizeInBytes(bigMessage); - assertThat(msgActualSize).isGreaterThan(sizeLimit); - } - - /** Individual Size check happy path special chars */ - @Test - public void checkForIndividualMessageSizeSpecialCharsLessThanLimit() { - AnalyticsClient client = newClient(); - int msgSize = MAX_MSG_SIZE; // 32KB - int sizeLimit = MAX_MSG_SIZE; // 32KB = 32768 - - Map properties = new HashMap(); - properties.put("property1", generateDataOfSizeSpecialChars(msgSize, true)); - - TrackMessage bigMessage = - TrackMessage.builder("Event").userId("jorgen25").properties(properties).build(); - client.enqueue(bigMessage); - - int msgActualSize = client.messageSizeInBytes(bigMessage); - assertThat(msgActualSize).isLessThanOrEqualTo(sizeLimit); - } - - /** Individual Size check sad path special chars (over the limit) */ - @Test - public void checkForIndividualMessageSizeSpecialCharsAboveLimit() { - AnalyticsClient client = newClient(); - int msgSize = MAX_MSG_SIZE; // 32KB - int sizeLimit = MAX_MSG_SIZE; // 32KB = 32768 - Map properties = new HashMap(); - - properties.put("property1", generateDataOfSizeSpecialChars(msgSize, false)); - - TrackMessage bigMessage = - TrackMessage.builder("Event").userId("jorgen25").properties(properties).build(); - - try { - client.enqueue(bigMessage); - } catch (IllegalArgumentException e) { - assertThat(e).isExactlyInstanceOf(e.getClass()); - } - - int msgActualSize = client.messageSizeInBytes(bigMessage); - assertThat(msgActualSize).isGreaterThan(sizeLimit); - } - - /** - * ***************************************************************************************************************** - * Test cases for enqueue modified logic - * *************************************************************************************************************** - */ - @Test - public void enqueueVerifyPoisonIsNotCheckedForSize() throws InterruptedException { - AnalyticsClient clientSpy = spy(newClient()); - - clientSpy.enqueue(POISON); - verify(messageQueue).put(POISON); - verify(clientSpy, never()).messageSizeInBytes(POISON); - } - - @Test - public void enqueueVerifyStopIsNotCheckedForSize() throws InterruptedException { - AnalyticsClient clientSpy = spy(newClient()); - - clientSpy.enqueue(STOP); - verify(messageQueue).put(STOP); - verify(clientSpy, never()).messageSizeInBytes(STOP); - } - - @Test - public void enqueueVerifyRegularMessageIsEnqueuedAndCheckedForSize(MessageBuilderTest builder) - throws InterruptedException { - AnalyticsClient clientSpy = spy(newClient()); - - Message message = builder.get().userId("jorgen25").build(); - clientSpy.enqueue(message); - verify(messageQueue).put(message); - verify(clientSpy, times(1)).messageSizeInBytes(message); - } - - /** - * This test case was to prove the limit in batch is not being respected so will probably delete - * it later NOTE: Used to be a test case created to prove huge messages above the limit are still - * being submitted in batch modified it to prove they are not anymore after changing logic in - * analyticsClient - * - * @param builder - * @throws InterruptedException - */ - @Test - public void enqueueSingleMessageAboveLimitWhenNotShutdown(MessageBuilderTest builder) - throws InterruptedException, IllegalArgumentException { - AnalyticsClient client = newClient(); - - // Message is above batch limit - final String massData = generateDataOfSizeSpecialChars(MAX_MSG_SIZE, false); - Map integrationOpts = new HashMap<>(); - integrationOpts.put("massData", massData); - Message message = - builder.get().userId("foo").integrationOptions("someKey", integrationOpts).build(); - - try { - client.enqueue(message); - } catch (IllegalArgumentException e) { - assertThat(e).isExactlyInstanceOf(e.getClass()); - } - - wait(messageQueue); - - // Message is above MSG/BATCH size limit so it should not be put in queue - verify(messageQueue, never()).put(message); - // And since it was never in the queue, it was never submitted in batch - verify(networkExecutor, never()).submit(any(AnalyticsClient.BatchUploadTask.class)); - } - - @Test - public void enqueueVerifyRegularMessagesSpecialCharactersBelowLimit(MessageBuilderTest builder) - throws InterruptedException, IllegalArgumentException { - AnalyticsClient client = newClient(); - int msgSize = 1024 * 18; // 18KB - - for (int i = 0; i < 2; i++) { - final String data = generateDataOfSizeSpecialChars(msgSize, true); - Map integrationOpts = new HashMap<>(); - integrationOpts.put("data", data); - Message message = - builder.get().userId("jorgen25").integrationOptions("someKey", integrationOpts).build(); - client.enqueue(message); - verify(messageQueue).put(message); - } - client.enqueue(POISON); - verify(messageQueue).put(POISON); - - wait(messageQueue); - client.shutdown(); - while (!isShutDown.get()) {} - - verify(networkExecutor, times(1)).submit(any(AnalyticsClient.BatchUploadTask.class)); - } - - /** - * ****************************************************************************************************************** - * Test cases for Batch creation logic - * **************************************************************************************************************** - */ - - /** - * Several messages are enqueued and then submitted in a batch - * - * @throws InterruptedException - */ - @Test - public void submitBatchBelowThreshold() throws InterruptedException, IllegalArgumentException { - AnalyticsClient client = - new AnalyticsClient( - messageQueue, - pendingQueue, - null, - segmentService, - 50, - TimeUnit.HOURS.toMillis(1), - 0, - MAX_BATCH_SIZE * 4, - log, - threadFactory, - networkExecutor, - isShutDown, - writeKey, - new Gson()); - - Map properties = new HashMap(); - properties.put("property3", generateDataOfSizeSpecialChars(((int) (MAX_MSG_SIZE * 0.9)), true)); - - for (int i = 0; i < 15; i++) { - TrackMessage bigMessage = - TrackMessage.builder("Big Event").userId("jorgen25").properties(properties).build(); - client.enqueue(bigMessage); - verify(messageQueue).put(bigMessage); - } - client.enqueue(POISON); - wait(messageQueue); - - client.shutdown(); - while (!isShutDown.get()) {} - verify(networkExecutor, times(1)).submit(any(Runnable.class)); - } - - /** - * Enqueued several messages above threshold of 500Kbs so queue gets backpressured at some point - * and several batches have to be created to not violate threshold - * - * @throws InterruptedException - */ - @Test - public void submitBatchAboveThreshold() throws InterruptedException, IllegalArgumentException { - AnalyticsClient client = - new AnalyticsClient( - messageQueue, - pendingQueue, - null, - segmentService, - 50, - TimeUnit.HOURS.toMillis(1), - 0, - MAX_BATCH_SIZE * 4, - log, - threadFactory, - networkExecutor, - isShutDown, - writeKey, - new Gson()); - - Map properties = new HashMap(); - properties.put("property3", generateDataOfSizeSpecialChars(MAX_MSG_SIZE, true)); - - for (int i = 0; i < 100; i++) { - TrackMessage message = - TrackMessage.builder("Big Event").userId("jorgen25").properties(properties).build(); - client.enqueue(message); - verify(messageQueue).put(message); - } - wait(messageQueue); - client.shutdown(); - while (!isShutDown.get()) {} - - verify(networkExecutor, times(8)).submit(any(Runnable.class)); - } - - @Test - public void submitManySmallMessagesBatchAboveThreshold() throws InterruptedException { - AnalyticsClient client = - new AnalyticsClient( - messageQueue, - pendingQueue, - null, - segmentService, - 50, - TimeUnit.HOURS.toMillis(1), - 0, - MAX_BATCH_SIZE * 4, - log, - threadFactory, - networkExecutor, - isShutDown, - writeKey, - new Gson()); - - Map properties = new HashMap(); - properties.put("property3", generateDataOfSizeSpecialChars(1024 * 8, true)); - - for (int i = 0; i < 600; i++) { - TrackMessage message = - TrackMessage.builder("Event").userId("jorgen25").properties(properties).build(); - client.enqueue(message); - verify(messageQueue).put(message); - } - wait(messageQueue); - client.shutdown(); - while (!isShutDown.get()) {} - - verify(networkExecutor, times(21)).submit(any(Runnable.class)); - } -} From ab780a4649d0885bf2cb1e5f82e984e90aaf87c1 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Tue, 25 Mar 2025 18:35:08 +0100 Subject: [PATCH 08/16] values --- .../analytics/internal/AnalyticsClient.java | 16 ++++---- .../analytics/internal/FallbackAppender.java | 5 +-- .../com/segment/analytics/SegmentTest.java | 37 +++++++++++++------ 3 files changed, 35 insertions(+), 23 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index 124d399a..fc31cc0d 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -119,14 +119,17 @@ public AnalyticsClient( looperThread.start(); CircuitBreaker> breaker = CircuitBreaker.>builder() - // 5 failure in 2 minute open the circuit - .withFailureThreshold(5, Duration.ofMinutes(2)) + // 10 failure in 2 minute open the circuit + .withFailureThreshold(10, Duration.ofMinutes(2)) // once open wait 30 seconds to be half-open .withDelay(Duration.ofSeconds(30)) // after 1 success the circuit is closed .withSuccessThreshold(1) // 5xx or rate limit is an error .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) + .onOpen(el -> System.err.println("***\nOPEN\n***")) + .onHalfOpen(el -> System.err.println("***\nHALF OPEN\n***")) + .onClose(el -> System.err.println("***\nCLOSED\n***")) .build(); RetryPolicy> retry = RetryPolicy.>builder() @@ -137,12 +140,6 @@ public AnalyticsClient( .handle(IOException.class) // retry on 5xx or rate limit .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) - .onRetriesExceeded(context -> { - throw new RuntimeException("retries"); - }) - .onAbort(context -> { - throw new RuntimeException("aborted"); - }) .build(); this.failsafe = Failsafe.with(retry, breaker).with(networkExecutor); @@ -167,6 +164,9 @@ public void enqueue(Message message) { if (!messageQueue.offer(message)) { handleError(message); } + else { + System.err.println("enqueued " + message.messageId()); + } } diff --git a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java index b80bdab4..2ee0ee10 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java +++ b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java @@ -69,6 +69,7 @@ public void close() { // block !!! public void add(Message msg) { try { + System.err.println("failed " + msg.messageId()); queue.put(msg); } catch (InterruptedException e) { Thread.currentThread().interrupt(); @@ -105,17 +106,15 @@ public void run() { // FIXME batch while (!msgs.isEmpty()) { - int reenqueued = 0; boolean canEnqueue = true; for (int i = msgs.size() - 1; canEnqueue && i >= 0; i--) { Message msg = msgs.get(i); canEnqueue = client.offer(msg); if (canEnqueue) { msgs.remove(i); - reenqueued++; + System.err.println("reenqueued " + msg.messageId()); } } - System.err.println("reenqueued " + reenqueued); try { Thread.sleep(1_000); } catch (InterruptedException e) { diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index be00286a..26474649 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -16,7 +16,10 @@ import java.util.Iterator; import java.util.List; import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import org.awaitility.Awaitility; import org.junit.After; import org.junit.Before; @@ -64,23 +67,30 @@ public void test() throws Throwable { .willReturn( WireMock.aResponse().withStatus(503).withBody("fail").withUniformRandomDelay(100, 1_000))); + int requestsPerSecond = 10; + int numClients = 10; + int timeToRun = 90_000; + int timeToRestore = 30_000; + long start = System.currentTimeMillis(); boolean upAgain = false; - int id = 0; + final AtomicInteger id = new AtomicInteger(0); List ids = new ArrayList<>(); - RateLimiter rate = RateLimiter.create(5); - while (System.currentTimeMillis() - start < 60_000) { + + RateLimiter rate = RateLimiter.create(requestsPerSecond); + ExecutorService exec = Executors.newWorkStealingPool(numClients); + + while (System.currentTimeMillis() - start < timeToRun) { if (rate.tryAcquire()) { - String msgid = "m" + id++; - ids.add(msgid); - analytics.enqueue( - TrackMessage.builder("my-track").messageId(msgid).userId("userId")); - System.err.println("enqued " + msgid); + exec.submit(() -> { + String msgid = "m" + id.getAndIncrement(); + ids.add(msgid); + analytics.enqueue( + TrackMessage.builder("my-track").messageId(msgid).userId("userId")); + }); } - - Thread.sleep(50); - - if (!upAgain && System.currentTimeMillis() - start > 20_000) { + Thread.sleep(1); + if (!upAgain && System.currentTimeMillis() - start > timeToRestore) { upAgain = true; stubFor(post(urlEqualTo("/v1/import/")) .willReturn(okJson("{\"success\": \"true\"}").withUniformRandomDelay(100, 1_000))); @@ -92,6 +102,9 @@ public void test() throws Throwable { .atMost(10, TimeUnit.MINUTES) .pollInterval(1, TimeUnit.SECONDS) .until(() -> sentMessagesEqualsTo(ids.toArray(new String[ids.size()]))); + + exec.shutdownNow(); + exec.awaitTermination(10, TimeUnit.SECONDS); } private static final ObjectMapper OM = new ObjectMapper(); From 2d06abfc5c80c6d0e60fcf1bc52481b3e16eae37 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Wed, 26 Mar 2025 14:37:16 +0100 Subject: [PATCH 09/16] doc --- ARQUITECTURE.md | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 ARQUITECTURE.md diff --git a/ARQUITECTURE.md b/ARQUITECTURE.md new file mode 100644 index 00000000..e06d772f --- /dev/null +++ b/ARQUITECTURE.md @@ -0,0 +1,60 @@ +```mermaid + +sequenceDiagram + box Data Consolidation + participant ConsolidationService + end + + participant SegmentClient + box HTTP + participant QueueHttp + participant LooperHttp + participant SegmentAPI + end + box File + participant QueueFile + participant WriteFile + participant File + participant WatchFile + end + + activate ConsolidationService + ConsolidationService->>+SegmentClient: enqueue + SegmentClient<<->>QueueHttp: offer + alt QueueHttp overflow + SegmentClient<<->>QueueFile: put + end + SegmentClient->>-ConsolidationService: + deactivate ConsolidationService + + loop consume QueueHttp + LooperHttp->>QueueHttp:take + activate LooperHttp + end + LooperHttp->>SegmentAPI: batchUpload + note over LooperHttp,SegmentAPI: Batch + note over LooperHttp,SegmentAPI: CircuitBreaker and Retry + note over LooperHttp,SegmentAPI: HTTP requests submited to a pool + deactivate LooperHttp + + alt retry exhausted or circuit open + note over LooperHttp: pool threads + LooperHttp->>QueueFile: put + end + + loop consume QueueFile + WriteFile->>QueueFile:take + activate WriteFile + end + WriteFile->>File: write + note over WriteFile: Batch and save file + deactivate WriteFile + + note over WatchFile: check last written + activate WatchFile + loop watch QueueFile + WatchFile->>File: read and remove + WatchFile->>QueueHttp: offer + end + deactivate WatchFile +``` From f73e156d013c4eb13faab4433dce04eaa40f1e30 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 31 Mar 2025 09:37:31 +0200 Subject: [PATCH 10/16] stole ReversedLinesFileReader at commons-io:2.18.0 --- analytics/pom.xml | 5 + .../internal/ReversedLinesFileReader.java | 541 ++++++++++++++++++ 2 files changed, 546 insertions(+) create mode 100644 analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java diff --git a/analytics/pom.xml b/analytics/pom.xml index 4315a511..8e61a10a 100644 --- a/analytics/pom.xml +++ b/analytics/pom.xml @@ -50,6 +50,11 @@ failsafe-retrofit 3.3.2 + + commons-io + commons-io + 2.18.0 + junit junit diff --git a/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java b/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java new file mode 100644 index 00000000..8ff1b35a --- /dev/null +++ b/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java @@ -0,0 +1,541 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.segment.analytics.internal; + +import java.io.Closeable; +import java.io.File; +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.nio.ByteBuffer; +import java.nio.channels.SeekableByteChannel; +import java.nio.charset.Charset; +import java.nio.charset.CharsetEncoder; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.apache.commons.io.Charsets; +import org.apache.commons.io.FileSystem; +import org.apache.commons.io.StandardLineSeparator; +import org.apache.commons.io.build.AbstractStreamBuilder; + +/** + * Reads lines in a file reversely (similar to a BufferedReader, but starting at the last line). Useful for e.g. searching in log files. + *

+ * To build an instance, use {@link Builder}. + *

+ * + * @see Builder + * @since 2.2 + */ +public class ReversedLinesFileReader implements Closeable { + + // @formatter:off + /** + * Builds a new {@link ReversedLinesFileReader}. + * + *

+ * For example: + *

+ * 
{@code
+     * ReversedLinesFileReader r = ReversedLinesFileReader.builder()
+     *   .setPath(path)
+     *   .setBufferSize(4096)
+     *   .setCharset(StandardCharsets.UTF_8)
+     *   .get();}
+     *
+ * + * @see #get() + * @since 2.12.0 + */ + // @formatter:on + public static class Builder extends AbstractStreamBuilder { + + /** + * Constructs a new {@link Builder}. + */ + public Builder() { + setBufferSizeDefault(DEFAULT_BLOCK_SIZE); + setBufferSize(DEFAULT_BLOCK_SIZE); + } + + /** + * Builds a new {@link ReversedLinesFileReader}. + *

+ * You must set input that supports {@link #getInputStream()} on this builder, otherwise, this method throws an exception. + *

+ *

+ * This builder use the following aspects: + *

+ *
    + *
  • {@link #getInputStream()}
    • + *
  • {@link #getBufferSize()}
    • + *
  • {@link #getCharset()}
    • + *
+ * + * @return a new instance. + * @throws IllegalStateException if the {@code origin} is {@code null}. + * @throws UnsupportedOperationException if the origin cannot be converted to a {@link Path}. + * @throws IOException if an I/O error occurs. + * @see #getPath() + * @see #getBufferSize() + * @see #getCharset() + */ + @Override + public ReversedLinesFileReader get() throws IOException { + return new ReversedLinesFileReader(getPath(), getBufferSize(), getCharset()); + } + } + + private final class FilePart { + private final long no; + + private final byte[] data; + + private byte[] leftOver; + + private int currentLastBytePos; + + /** + * Constructs a new instance. + * + * @param no the part number + * @param length its length + * @param leftOverOfLastFilePart remainder + * @throws IOException if there is a problem reading the file + */ + private FilePart(final long no, final int length, final byte[] leftOverOfLastFilePart) throws IOException { + this.no = no; + final int dataLength = length + (leftOverOfLastFilePart != null ? leftOverOfLastFilePart.length : 0); + this.data = new byte[dataLength]; + final long off = (no - 1) * blockSize; + + // read data + if (no > 0 /* file not empty */) { + channel.position(off); + final int countRead = channel.read(ByteBuffer.wrap(data, 0, length)); + if (countRead != length) { + throw new IllegalStateException("Count of requested bytes and actually read bytes don't match"); + } + } + // copy left over part into data arr + if (leftOverOfLastFilePart != null) { + System.arraycopy(leftOverOfLastFilePart, 0, data, length, leftOverOfLastFilePart.length); + } + this.currentLastBytePos = data.length - 1; + this.leftOver = null; + } + + /** + * Constructs the buffer containing any leftover bytes. + */ + private void createLeftOver() { + final int lineLengthBytes = currentLastBytePos + 1; + if (lineLengthBytes > 0) { + // create left over for next block + leftOver = Arrays.copyOf(data, lineLengthBytes); + } else { + leftOver = null; + } + currentLastBytePos = -1; + } + + /** + * Finds the new-line sequence and return its length. + * + * @param data buffer to scan + * @param i start offset in buffer + * @return length of newline sequence or 0 if none found + */ + private int getNewLineMatchByteCount(final byte[] data, final int i) { + for (final byte[] newLineSequence : newLineSequences) { + boolean match = true; + for (int j = newLineSequence.length - 1; j >= 0; j--) { + final int k = i + j - (newLineSequence.length - 1); + match &= k >= 0 && data[k] == newLineSequence[j]; + } + if (match) { + return newLineSequence.length; + } + } + return 0; + } + + /** + * Reads a line. + * + * @return the line or null + */ + private String readLine() { // NOPMD Bug in PMD + + String line = null; + int newLineMatchByteCount; + + final boolean isLastFilePart = no == 1; + + int i = currentLastBytePos; + while (i > -1) { + + if (!isLastFilePart && i < avoidNewlineSplitBufferSize) { + // avoidNewlineSplitBuffer: for all except the last file part we + // take a few bytes to the next file part to avoid splitting of newlines + createLeftOver(); + break; // skip last few bytes and leave it to the next file part + } + + // check for newline + if ((newLineMatchByteCount = getNewLineMatchByteCount(data, i)) > 0 /* found newline */) { + final int lineStart = i + 1; + final int lineLengthBytes = currentLastBytePos - lineStart + 1; + + if (lineLengthBytes < 0) { + throw new IllegalStateException("Unexpected negative line length=" + lineLengthBytes); + } + final byte[] lineData = Arrays.copyOfRange(data, lineStart, lineStart + lineLengthBytes); + + line = new String(lineData, charset); + + currentLastBytePos = i - newLineMatchByteCount; + break; // found line + } + + // move cursor + i -= byteDecrement; + + // end of file part handling + if (i < 0) { + createLeftOver(); + break; // end of file part + } + } + + // last file part handling + if (isLastFilePart && leftOver != null) { + // there will be no line break anymore, this is the first line of the file + line = new String(leftOver, charset); + leftOver = null; + } + + return line; + } + + /** + * Handles block rollover + * + * @return the new FilePart or null + * @throws IOException if there was a problem reading the file + */ + private FilePart rollOver() throws IOException { + + if (currentLastBytePos > -1) { + throw new IllegalStateException("Current currentLastCharPos unexpectedly positive... " + + "last readLine() should have returned something! currentLastCharPos=" + currentLastBytePos); + } + + if (no > 1) { + return new FilePart(no - 1, blockSize, leftOver); + } + // NO 1 was the last FilePart, we're finished + if (leftOver != null) { + throw new IllegalStateException("Unexpected leftover of the last block: leftOverOfThisFilePart=" + + new String(leftOver, charset)); + } + return null; + } + } + + private static final String EMPTY_STRING = ""; + + private static final int DEFAULT_BLOCK_SIZE = FileSystem.getCurrent().getBlockSize(); + + /** + * Constructs a new {@link Builder}. + * + * @return a new {@link Builder}. + * @since 2.12.0 + */ + public static Builder builder() { + return new Builder(); + } + + private final int blockSize; + private final Charset charset; + private final SeekableByteChannel channel; + private final long totalByteLength; + private final long totalBlockCount; + private final byte[][] newLineSequences; + private final int avoidNewlineSplitBufferSize; + private final int byteDecrement; + private FilePart currentFilePart; + private boolean trailingNewlineOfFileSkipped; + + /** + * Constructs a ReversedLinesFileReader with default block size of 4KB and the + * platform's default encoding. + * + * @param file the file to be read + * @throws IOException if an I/O error occurs. + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final File file) throws IOException { + this(file, DEFAULT_BLOCK_SIZE, Charset.defaultCharset()); + } + + /** + * Constructs a ReversedLinesFileReader with default block size of 4KB and the + * specified encoding. + * + * @param file the file to be read + * @param charset the charset to use, null uses the default Charset. + * @throws IOException if an I/O error occurs. + * @since 2.5 + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final File file, final Charset charset) throws IOException { + this(file.toPath(), charset); + } + + /** + * Constructs a ReversedLinesFileReader with the given block size and encoding. + * + * @param file the file to be read + * @param blockSize size of the internal buffer (for ideal performance this + * should match with the block size of the underlying file + * system). + * @param charset the encoding of the file, null uses the default Charset. + * @throws IOException if an I/O error occurs. + * @since 2.3 + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final File file, final int blockSize, final Charset charset) throws IOException { + this(file.toPath(), blockSize, charset); + } + + /** + * Constructs a ReversedLinesFileReader with the given block size and encoding. + * + * @param file the file to be read + * @param blockSize size of the internal buffer (for ideal performance this + * should match with the block size of the underlying file + * system). + * @param charsetName the encoding of the file, null uses the default Charset. + * @throws IOException if an I/O error occurs + * @throws java.nio.charset.UnsupportedCharsetException if the encoding is not supported + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final File file, final int blockSize, final String charsetName) throws IOException { + this(file.toPath(), blockSize, charsetName); + } + + /** + * Constructs a ReversedLinesFileReader with default block size of 4KB and the + * specified encoding. + * + * @param file the file to be read + * @param charset the charset to use, null uses the default Charset. + * @throws IOException if an I/O error occurs. + * @since 2.7 + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final Path file, final Charset charset) throws IOException { + this(file, DEFAULT_BLOCK_SIZE, charset); + } + + /** + * Constructs a ReversedLinesFileReader with the given block size and encoding. + * + * @param file the file to be read + * @param blockSize size of the internal buffer (for ideal performance this + * should match with the block size of the underlying file + * system). + * @param charset the encoding of the file, null uses the default Charset. + * @throws IOException if an I/O error occurs. + * @since 2.7 + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final Path file, final int blockSize, final Charset charset) throws IOException { + this.blockSize = blockSize; + this.charset = Charsets.toCharset(charset); + + // --- check & prepare encoding --- + final CharsetEncoder charsetEncoder = this.charset.newEncoder(); + final float maxBytesPerChar = charsetEncoder.maxBytesPerChar(); + if (maxBytesPerChar == 1f || this.charset == StandardCharsets.UTF_8) { + // all one byte encodings are no problem + byteDecrement = 1; + } else if (this.charset == Charset.forName("Shift_JIS") + || // Same as for UTF-8 + // http://www.herongyang.com/Unicode/JIS-Shift-JIS-Encoding.html + this.charset == Charset.forName("windows-31j") + || // Windows code page 932 (Japanese) + this.charset == Charset.forName("x-windows-949") + || // Windows code page 949 (Korean) + this.charset == Charset.forName("gbk") + || // Windows code page 936 (Simplified Chinese) + this.charset == Charset.forName("x-windows-950")) { // Windows code page 950 (Traditional Chinese) + byteDecrement = 1; + } else if (this.charset == StandardCharsets.UTF_16BE || this.charset == StandardCharsets.UTF_16LE) { + // UTF-16 new line sequences are not allowed as second tuple of four byte + // sequences, + // however byte order has to be specified + byteDecrement = 2; + } else if (this.charset == StandardCharsets.UTF_16) { + throw new UnsupportedEncodingException( + "For UTF-16, you need to specify the byte order (use UTF-16BE or " + "UTF-16LE)"); + } else { + throw new UnsupportedEncodingException( + "Encoding " + charset + " is not supported yet (feel free to " + "submit a patch)"); + } + + // NOTE: The new line sequences are matched in the order given, so it is + // important that \r\n is BEFORE \n + this.newLineSequences = new byte[][] { + StandardLineSeparator.CRLF.getBytes(this.charset), + StandardLineSeparator.LF.getBytes(this.charset), + StandardLineSeparator.CR.getBytes(this.charset) + }; + + this.avoidNewlineSplitBufferSize = newLineSequences[0].length; + + // Open file + this.channel = Files.newByteChannel(file, StandardOpenOption.READ); + this.totalByteLength = channel.size(); + int lastBlockLength = (int) (this.totalByteLength % blockSize); + if (lastBlockLength > 0) { + this.totalBlockCount = this.totalByteLength / blockSize + 1; + } else { + this.totalBlockCount = this.totalByteLength / blockSize; + if (this.totalByteLength > 0) { + lastBlockLength = blockSize; + } + } + this.currentFilePart = new FilePart(totalBlockCount, lastBlockLength, null); + } + + /** + * Constructs a ReversedLinesFileReader with the given block size and encoding. + * + * @param file the file to be read + * @param blockSize size of the internal buffer (for ideal performance this + * should match with the block size of the underlying file + * system). + * @param charsetName the encoding of the file, null uses the default Charset. + * @throws IOException if an I/O error occurs + * @throws java.nio.charset.UnsupportedCharsetException if the encoding is not supported + * @since 2.7 + * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} + */ + @Deprecated + public ReversedLinesFileReader(final Path file, final int blockSize, final String charsetName) throws IOException { + this(file, blockSize, Charsets.toCharset(charsetName)); + } + + /** + * Closes underlying resources. + * + * @throws IOException if an I/O error occurs. + */ + @Override + public void close() throws IOException { + channel.close(); + } + + /** + * Returns the lines of the file from bottom to top. + * + * @return the next line or null if the start of the file is reached + * @throws IOException if an I/O error occurs. + */ + public String readLine() throws IOException { + + String line = currentFilePart.readLine(); + while (line == null) { + currentFilePart = currentFilePart.rollOver(); + if (currentFilePart == null) { + // no more FileParts: we're done, leave line set to null + break; + } + line = currentFilePart.readLine(); + } + + // aligned behavior with BufferedReader that doesn't return a last, empty line + if (EMPTY_STRING.equals(line) && !trailingNewlineOfFileSkipped) { + trailingNewlineOfFileSkipped = true; + line = readLine(); + } + + return line; + } + + /** + * Returns {@code lineCount} lines of the file from bottom to top. + *

+ * If there are less than {@code lineCount} lines in the file, then that's what + * you get. + *

+ *

+ * Note: You can easily flip the result with {@link Collections#reverse(List)}. + *

+ * + * @param lineCount How many lines to read. + * @return A new list + * @throws IOException if an I/O error occurs. + * @since 2.8.0 + */ + public List readLines(final int lineCount) throws IOException { + if (lineCount < 0) { + throw new IllegalArgumentException("lineCount < 0"); + } + final ArrayList arrayList = new ArrayList<>(lineCount); + for (int i = 0; i < lineCount; i++) { + final String line = readLine(); + if (line == null) { + return arrayList; + } + arrayList.add(line); + } + return arrayList; + } + + /** + * Returns the last {@code lineCount} lines of the file. + *

+ * If there are less than {@code lineCount} lines in the file, then that's what + * you get. + *

+ * + * @param lineCount How many lines to read. + * @return A String. + * @throws IOException if an I/O error occurs. + * @since 2.8.0 + */ + public String toString(final int lineCount) throws IOException { + final List lines = readLines(lineCount); + Collections.reverse(lines); + return lines.isEmpty() ? EMPTY_STRING : String.join(System.lineSeparator(), lines) + System.lineSeparator(); + } +} From 0d2527e51861969230463b7fe15becdfe9dc3710 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 31 Mar 2025 09:37:42 +0200 Subject: [PATCH 11/16] ReversedLinesFileReader truncate consumed lines --- .../internal/ReversedLinesFileReader.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java b/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java index 8ff1b35a..09a7a773 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java +++ b/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java @@ -21,11 +21,11 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.nio.ByteBuffer; -import java.nio.channels.SeekableByteChannel; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; import java.nio.charset.Charset; import java.nio.charset.CharsetEncoder; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ArrayList; @@ -278,7 +278,8 @@ public static Builder builder() { private final int blockSize; private final Charset charset; - private final SeekableByteChannel channel; + private final FileChannel channel; + private final FileLock fileLock; private final long totalByteLength; private final long totalBlockCount; private final byte[][] newLineSequences; @@ -422,7 +423,8 @@ public ReversedLinesFileReader(final Path file, final int blockSize, final Chars this.avoidNewlineSplitBufferSize = newLineSequences[0].length; // Open file - this.channel = Files.newByteChannel(file, StandardOpenOption.READ); + this.channel = FileChannel.open(file, StandardOpenOption.READ, StandardOpenOption.WRITE); + this.fileLock = channel.lock(); this.totalByteLength = channel.size(); int lastBlockLength = (int) (this.totalByteLength % blockSize); if (lastBlockLength > 0) { @@ -461,6 +463,7 @@ public ReversedLinesFileReader(final Path file, final int blockSize, final Strin */ @Override public void close() throws IOException { + fileLock.release(); channel.close(); } @@ -514,10 +517,18 @@ public List readLines(final int lineCount) throws IOException { for (int i = 0; i < lineCount; i++) { final String line = readLine(); if (line == null) { + channel.truncate(0); return arrayList; } arrayList.add(line); } + + long truncateTo = (this.currentFilePart.no - 1) * blockSize; + truncateTo += this.currentFilePart.currentLastBytePos + 1; + channel.truncate(truncateTo); + + channel.force(true); + return arrayList; } From 389875f6cd8525d9edf7959df67c83b933f44817 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 31 Mar 2025 09:37:48 +0200 Subject: [PATCH 12/16] truncate lines on resend --- .../analytics/internal/FallbackAppender.java | 77 +++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java index 2ee0ee10..1f85e392 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java +++ b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java @@ -11,10 +11,10 @@ import java.io.OutputStream; import java.nio.channels.Channels; import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; import java.nio.file.StandardOpenOption; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.List; @@ -25,6 +25,7 @@ import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; +import org.apache.commons.io.FileSystem; public class FallbackAppender { @@ -86,25 +87,18 @@ public void run() { } List msgs; - lock.lock(); try { - msgs = read(); + msgs = truncate(20); // XXX messageSize if (msgs.isEmpty()) { continue; } - // FIXME now its reading all the msgs and waits until all is processed - // it will be better to work with batch and truncate the file - file.delete(); } catch (IOException e) { // TODO Auto-generated catch block e.printStackTrace(); lastMessage = System.currentTimeMillis(); continue; - } finally { - lock.unlock(); } - // FIXME batch while (!msgs.isEmpty()) { boolean canEnqueue = true; for (int i = msgs.size() - 1; canEnqueue && i >= 0; i--) { @@ -113,16 +107,16 @@ public void run() { if (canEnqueue) { msgs.remove(i); System.err.println("reenqueued " + msg.messageId()); + } else { + // slow down next iteration when http queue overflow + try { + Thread.sleep(1_000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } } - try { - Thread.sleep(1_000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } } - - lastMessage = System.currentTimeMillis(); } try { @@ -161,38 +155,43 @@ public void run() { } } - List read() throws IOException { - if (file.exists()) { - try (FileChannel fileChannel = FileChannel.open(file.toPath(), StandardOpenOption.READ)) { - fileChannel.lock(0, Long.MAX_VALUE, true); - - final String[] lines = new String( - Channels.newInputStream(fileChannel).readAllBytes(), StandardCharsets.UTF_8) - .split(System.lineSeparator()); - return Arrays.stream(lines) - .map(m -> fromJson(m)) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - } - } else { + List truncate(int numMessages) throws IOException { + lock.lock(); + + if (!file.exists()) { + lock.unlock(); return Collections.emptyList(); } + + try (ReversedLinesFileReader reader = ReversedLinesFileReader.builder() + .setPath(file.toPath()) + .setBufferSize(FileSystem.getCurrent().getBlockSize()) + .setCharset(StandardCharsets.UTF_8) + .get()) { + + return reader.readLines(numMessages).stream() + .map(this::fromJson) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + } finally { + lock.unlock(); + } } + private static final byte[] NEW_LINE = System.lineSeparator().getBytes(StandardCharsets.UTF_8); + private void write(List batch) { lock.lock(); try (FileChannel fileChannel = FileChannel.open( - file.toPath(), StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE)) { - fileChannel.lock(); + file.toPath(), StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE); + OutputStream os = Channels.newOutputStream(fileChannel); + FileLock fileLock = fileChannel.lock(); ) { - final String lines = batch.stream() - .map(this::toJson) - .filter(Objects::nonNull) - .collect(Collectors.joining(System.lineSeparator())); + for (Message msg : batch) { + os.write(toJson(msg).getBytes(StandardCharsets.UTF_8)); + os.write(NEW_LINE); + } - OutputStream os = Channels.newOutputStream(fileChannel); - os.write(lines.getBytes(StandardCharsets.UTF_8)); - os.write(System.lineSeparator().getBytes(StandardCharsets.UTF_8)); fileChannel.force(true); batch.clear(); From ac0bc1fed9492f0c16ca3aa75842e3d8ef42fa76 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 31 Mar 2025 16:54:36 +0200 Subject: [PATCH 13/16] stop retry on rate limit --- .../com/segment/analytics/internal/AnalyticsClient.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index fc31cc0d..981a948f 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -138,8 +138,10 @@ public AnalyticsClient( .withJitter(.2) // retry on IOException .handle(IOException.class) - // retry on 5xx or rate limit - .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) + // retry on 5xx + .handleResultIf(response -> is5xx(response.code())) + // stop retry on rate limit + .abortIf(response -> response.code() == 429) .build(); this.failsafe = Failsafe.with(retry, breaker).with(networkExecutor); From e100ede291c6a885678313bd3a87121e161cd409 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Tue, 1 Apr 2025 16:59:32 +0200 Subject: [PATCH 14/16] store small batch size --- .../analytics/http/SegmentService.java | 4 + .../java/com/segment/analytics/Analytics.java | 119 ++-- .../java/com/segment/analytics/Platform.java | 62 -- .../analytics/internal/AnalyticsClient.java | 469 ++++++++------- .../segment/analytics/internal/Config.java | 185 ++++++ .../analytics/internal/FallbackAppender.java | 400 +++++++------ .../analytics/internal/ResubmitCheck.java | 60 ++ .../internal/ReversedLinesFileReader.java | 552 ------------------ .../analytics/AnalyticsBuilderTest.java | 429 -------------- .../com/segment/analytics/SegmentTest.java | 127 +++- 10 files changed, 834 insertions(+), 1573 deletions(-) delete mode 100644 analytics/src/main/java/com/segment/analytics/Platform.java create mode 100644 analytics/src/main/java/com/segment/analytics/internal/Config.java create mode 100644 analytics/src/main/java/com/segment/analytics/internal/ResubmitCheck.java delete mode 100644 analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java delete mode 100644 analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java diff --git a/analytics-core/src/main/java/com/segment/analytics/http/SegmentService.java b/analytics-core/src/main/java/com/segment/analytics/http/SegmentService.java index c96cbef2..95389759 100644 --- a/analytics-core/src/main/java/com/segment/analytics/http/SegmentService.java +++ b/analytics-core/src/main/java/com/segment/analytics/http/SegmentService.java @@ -2,6 +2,7 @@ import com.segment.analytics.messages.Batch; import okhttp3.HttpUrl; +import okhttp3.RequestBody; import retrofit2.Call; import retrofit2.http.Body; import retrofit2.http.POST; @@ -11,4 +12,7 @@ public interface SegmentService { @POST Call upload(@Url HttpUrl uploadUrl, @Body Batch batch); + + @POST + Call upload(@Url HttpUrl uploadUrl, @Body RequestBody batch); } diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java index bccce351..67344ae2 100644 --- a/analytics/src/main/java/com/segment/analytics/Analytics.java +++ b/analytics/src/main/java/com/segment/analytics/Analytics.java @@ -7,8 +7,13 @@ import com.segment.analytics.http.SegmentService; import com.segment.analytics.internal.AnalyticsClient; import com.segment.analytics.internal.AnalyticsVersion; +import com.segment.analytics.internal.Config; +import com.segment.analytics.internal.Config.FileConfig; +import com.segment.analytics.internal.Config.HttpConfig; import com.segment.analytics.messages.Message; import com.segment.analytics.messages.MessageBuilder; +import java.io.Closeable; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -16,7 +21,6 @@ import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import okhttp3.ConnectionSpec; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; @@ -40,7 +44,7 @@ * * @see Segment */ -public class Analytics { +public class Analytics implements Closeable { private final AnalyticsClient client; private final List messageTransformers; private final List messageInterceptors; @@ -90,9 +94,9 @@ public boolean offer(MessageBuilder builder) { return client.offer(message); } - /** Stops this instance from processing further requests. */ - public void shutdown() { - client.shutdown(); + /** Stops this instance from processing further requests. */ + public void close() { + client.close(); } /** @@ -125,7 +129,6 @@ public static class Builder { private static final String DEFAULT_ENDPOINT = "https://api.segment.io"; private static final String DEFAULT_PATH = "/v1/import/"; private static final String DEFAULT_USER_AGENT = "analytics-java/" + AnalyticsVersion.get(); - private static final int MESSAGE_QUEUE_MAX_BYTE_SIZE = 1024 * 500; private final String writeKey; private OkHttpClient client; @@ -137,12 +140,10 @@ public static class Builder { private List messageInterceptors; private ExecutorService networkExecutor; private ThreadFactory threadFactory; - private int flushQueueSize; - private int maximumFlushAttempts; - private long flushIntervalInMillis; - private int queueCapacity; private boolean forceTlsV1 = false; private GsonBuilder gsonBuilder; + private HttpConfig httpConfig; + private FileConfig fileConfig; Builder(String writeKey) { if (writeKey == null || writeKey.trim().length() == 0) { @@ -234,15 +235,6 @@ public Builder messageInterceptor(MessageInterceptor interceptor) { return this; } - /** Set queue capacity */ - public Builder queueCapacity(int capacity) { - if (capacity <= 0) { - throw new IllegalArgumentException("capacity should be positive."); - } - this.queueCapacity = capacity; - return this; - } - public Builder gsonBuilder(GsonBuilder gsonBuilder) { if (gsonBuilder == null) { throw new NullPointerException("Null gsonBuilder"); @@ -256,36 +248,6 @@ public Builder gsonBuilder(GsonBuilder gsonBuilder) { return this; } - /** Set the queueSize at which flushes should be triggered. */ - @Beta - public Builder flushQueueSize(int flushQueueSize) { - if (flushQueueSize < 1) { - throw new IllegalArgumentException("flushQueueSize must not be less than 1."); - } - this.flushQueueSize = flushQueueSize; - return this; - } - - /** Set the interval at which the queue should be flushed. */ - @Beta - public Builder flushInterval(long flushInterval, TimeUnit unit) { - long flushIntervalInMillis = unit.toMillis(flushInterval); - if (flushIntervalInMillis < 1000) { - throw new IllegalArgumentException("flushInterval must not be less than 1 second."); - } - this.flushIntervalInMillis = flushIntervalInMillis; - return this; - } - - /** Set how many retries should happen before getting exhausted */ - public Builder retries(int maximumRetries) { - if (maximumRetries < 1) { - throw new IllegalArgumentException("retries must be at least 1"); - } - this.maximumFlushAttempts = maximumRetries; - return this; - } - /** Set the {@link ExecutorService} on which all HTTP requests will be made. */ public Builder networkExecutor(ExecutorService networkExecutor) { if (networkExecutor == null) { @@ -320,9 +282,22 @@ public Builder forceTlsVersion1() { forceTlsV1 = true; return this; } + + public Builder httpConfig(HttpConfig httpConfig) { + this.httpConfig = httpConfig; + return this; + } + public Builder fileConfig(FileConfig fileConfig) { + this.fileConfig = fileConfig; + return this; + } - /** Create a {@link Analytics} client. */ - public Analytics build() { + /** + * Create a {@link Analytics} client. + * + * @throws IOException if cannot create the configured filePath directory + */ + public Analytics build() throws IOException { if (gsonBuilder == null) { gsonBuilder = new GsonBuilder(); } @@ -341,25 +316,9 @@ public Analytics build() { } } - if (client == null) { - client = Platform.get().defaultClient(); - } - if (log == null) { log = Log.NONE; } - if (flushIntervalInMillis == 0) { - flushIntervalInMillis = Platform.get().defaultFlushIntervalInMillis(); - } - if (queueCapacity == 0) { - queueCapacity = Integer.MAX_VALUE; - } - if (flushQueueSize == 0) { - flushQueueSize = Platform.get().defaultFlushQueueSize(); - } - if (maximumFlushAttempts == 0) { - maximumFlushAttempts = 3; - } if (messageTransformers == null) { messageTransformers = Collections.emptyList(); } else { @@ -371,10 +330,19 @@ public Analytics build() { messageInterceptors = Collections.unmodifiableList(messageInterceptors); } if (networkExecutor == null) { - networkExecutor = Platform.get().defaultNetworkExecutor(); + networkExecutor = Config.defaultNetworkExecutor(); } if (threadFactory == null) { - threadFactory = Platform.get().defaultThreadFactory(); + threadFactory = Config.defaultThreadFactory(); + } + if (client == null) { + client = Config.defaultClient(); + } + if(httpConfig == null) { + httpConfig = HttpConfig.builder().build(); + } + if(fileConfig == null) { + fileConfig = FileConfig.builder().build(); } HttpLoggingInterceptor interceptor = @@ -415,18 +383,7 @@ public void log(String message) { SegmentService segmentService = restAdapter.create(SegmentService.class); - AnalyticsClient analyticsClient = AnalyticsClient.create( - endpoint, - segmentService, - queueCapacity, - flushQueueSize, - flushIntervalInMillis, - log, - threadFactory, - networkExecutor, - writeKey, - gson); - + AnalyticsClient analyticsClient = new AnalyticsClient(endpoint, segmentService, log, threadFactory, networkExecutor, writeKey, gson, httpConfig, fileConfig); return new Analytics(analyticsClient, messageTransformers, messageInterceptors, log); } } diff --git a/analytics/src/main/java/com/segment/analytics/Platform.java b/analytics/src/main/java/com/segment/analytics/Platform.java deleted file mode 100644 index a5de5cb5..00000000 --- a/analytics/src/main/java/com/segment/analytics/Platform.java +++ /dev/null @@ -1,62 +0,0 @@ -package com.segment.analytics; - -import static java.lang.Thread.MIN_PRIORITY; - -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import okhttp3.OkHttpClient; - -class Platform { - static final String THREAD_NAME = "Analytics"; - - private static final Platform PLATFORM = findPlatform(); - - static Platform get() { - return PLATFORM; - } - - private static Platform findPlatform() { - return new Platform(); - } - - OkHttpClient defaultClient() { - OkHttpClient client = - new OkHttpClient.Builder() - .connectTimeout(15, TimeUnit.SECONDS) - .readTimeout(15, TimeUnit.SECONDS) - .writeTimeout(15, TimeUnit.SECONDS) - .build(); - return client; - } - - ExecutorService defaultNetworkExecutor() { - return Executors.newSingleThreadExecutor(defaultThreadFactory()); - } - - ThreadFactory defaultThreadFactory() { - return new ThreadFactory() { - @Override - public Thread newThread(final Runnable r) { - return new Thread( - new Runnable() { - @Override - public void run() { - Thread.currentThread().setPriority(MIN_PRIORITY); - r.run(); - } - }, - THREAD_NAME); - } - }; - } - - public long defaultFlushIntervalInMillis() { - return 10 * 1000; // 10s - } - - public int defaultFlushQueueSize() { - return 250; - } -} diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index 981a948f..2ca27085 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -8,19 +8,18 @@ import com.segment.analytics.Log; import com.segment.analytics.http.SegmentService; import com.segment.analytics.http.UploadResponse; +import com.segment.analytics.internal.Config.FileConfig; +import com.segment.analytics.internal.Config.HttpConfig; import com.segment.analytics.messages.Batch; import com.segment.analytics.messages.Message; import dev.failsafe.CircuitBreaker; -import dev.failsafe.CircuitBreakerOpenException; -import dev.failsafe.Failsafe; -import dev.failsafe.FailsafeExecutor; -import dev.failsafe.RetryPolicy; -import dev.failsafe.retrofit.FailsafeCall; +import java.io.Closeable; import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.time.Duration; -import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; @@ -28,22 +27,28 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.BlockingQueue; -import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Level; +import java.util.logging.Logger; import okhttp3.HttpUrl; -import retrofit2.Call; +import okhttp3.MediaType; +import okhttp3.RequestBody; import retrofit2.Response; -public class AnalyticsClient { +public class AnalyticsClient implements Closeable { + private static final Logger LOGGER = Logger.getLogger(AnalyticsClient.class.getName()); + private static final Map CONTEXT; private static final int BATCH_MAX_SIZE = 1024 * 500; + private static final int MSG_MAX_SIZE = 1024 * 32; private static final Charset ENCODING = StandardCharsets.UTF_8; private Gson gsonInstance; - private static final String instanceId = UUID.randomUUID().toString(); // TODO configurable ? + private static final String instanceId = UUID.randomUUID().toString(); // TODO configurable ? static { Map library = new LinkedHashMap<>(); @@ -55,149 +60,99 @@ public class AnalyticsClient { CONTEXT = Collections.unmodifiableMap(context); } + private final HttpConfig config; private final BlockingQueue messageQueue; private final HttpUrl uploadUrl; private final SegmentService service; - private final int flushQueueSize; - private final long flushIntervalInMillis; private final Log log; private final ExecutorService networkExecutor; - private final Thread looperThread; - private final AtomicBoolean isShutDown; private final String writeKey; - private final FailsafeExecutor> failsafe; - private final FallbackAppender fallback; - - public static AnalyticsClient create( - HttpUrl uploadUrl, - SegmentService segmentService, - int queueCapacity, - int flushQueueSize, - long flushIntervalInMillis, - Log log, - ThreadFactory threadFactory, - ExecutorService networkExecutor, - String writeKey, - Gson gsonInstance) { - return new AnalyticsClient( - new LinkedBlockingQueue(queueCapacity), - uploadUrl, - segmentService, - flushQueueSize, - flushIntervalInMillis, - log, - threadFactory, - networkExecutor, - new AtomicBoolean(false), - writeKey, - gsonInstance); - } - - public AnalyticsClient( - BlockingQueue messageQueue, - HttpUrl uploadUrl, - SegmentService service, - int flushQueueSize, - long flushIntervalInMillis, - Log log, - ThreadFactory threadFactory, - ExecutorService networkExecutor, - AtomicBoolean isShutDown, - String writeKey, - Gson gsonInstance) { - this.messageQueue = messageQueue; + private final Thread looperThread; + private final AtomicBoolean isShutDown = new AtomicBoolean(false); + private final CircuitBreaker breaker; + private final FallbackAppender fallback; + private final ResubmitCheck resubmit; + + public AnalyticsClient(HttpUrl uploadUrl, SegmentService service, Log log, ThreadFactory threadFactory, + ExecutorService networkExecutor, String writeKey, Gson gsonInstance, HttpConfig config, FileConfig fileConfig) + throws IOException { + this.config = config; + this.messageQueue = new LinkedBlockingQueue(config.queueSize); this.uploadUrl = uploadUrl; this.service = service; - this.flushQueueSize = flushQueueSize; - this.flushIntervalInMillis = flushIntervalInMillis; this.log = log; this.looperThread = threadFactory.newThread(new Looper()); + this.looperThread.setName(AnalyticsClient.class.getSimpleName() + "-Looper"); this.networkExecutor = networkExecutor; - this.isShutDown = isShutDown; this.writeKey = writeKey; this.gsonInstance = gsonInstance; - looperThread.start(); - - CircuitBreaker> breaker = CircuitBreaker.>builder() - // 10 failure in 2 minute open the circuit - .withFailureThreshold(10, Duration.ofMinutes(2)) - // once open wait 30 seconds to be half-open - .withDelay(Duration.ofSeconds(30)) - // after 1 success the circuit is closed - .withSuccessThreshold(1) - // 5xx or rate limit is an error - .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) - .onOpen(el -> System.err.println("***\nOPEN\n***")) - .onHalfOpen(el -> System.err.println("***\nHALF OPEN\n***")) - .onClose(el -> System.err.println("***\nCLOSED\n***")) - .build(); - - RetryPolicy> retry = RetryPolicy.>builder() - .withMaxAttempts(5) - .withBackoff(1, 300, ChronoUnit.SECONDS) - .withJitter(.2) - // retry on IOException - .handle(IOException.class) - // retry on 5xx - .handleResultIf(response -> is5xx(response.code())) - // stop retry on rate limit - .abortIf(response -> response.code() == 429) - .build(); - - this.failsafe = Failsafe.with(retry, breaker).with(networkExecutor); - this.fallback = new FallbackAppender(this); + + this.breaker = CircuitBreaker.>builder() + // X failure in 1 minute open the circuit + .withFailureThreshold(config.circuitErrorsInAMinute, Duration.ofMinutes(1)) + // once open wait X seconds to be half-open + .withDelay(Duration.ofSeconds(config.circuitSecondsInOpen)) + // after X success the circuit is closed + .withSuccessThreshold(config.circuitRequestToClose) + // 5xx or rate limit is an error + .handleResultIf(response -> is5xx(response.code()) || response.code() == 429) + .onOpen(el -> LOGGER.log(Level.INFO, "OPEN: failing requests")) + .onHalfOpen(el -> LOGGER.log(Level.INFO, "HALF OPEN: checking status")) + .onClose(el -> LOGGER.log(Level.INFO, "CLOSED: attending requests normally")).build(); + + this.fallback = new FallbackAppender(gsonInstance, threadFactory, fileConfig); + this.resubmit = new ResubmitCheck(threadFactory, fileConfig, this); + + looperThread.start(); } public int messageSizeInBytes(Message message) { String stringifiedMessage = gsonInstance.toJson(message); - return stringifiedMessage.getBytes(ENCODING).length; } - public boolean offer(Message message) { + public boolean offer(Message message) throws IllegalArgumentException { + if (messageSizeInBytes(message) > MSG_MAX_SIZE) { + throw new IllegalArgumentException("Message was above individual limit. MessageId: " + message.messageId()); + } + return messageQueue.offer(message); } - public void enqueue(Message message) { - if (isShutDown.get()) { - log.print(ERROR, "Attempt to enqueue a message when shutdown has been called %s.", message); - return; - } - if (!messageQueue.offer(message)) { - handleError(message); - } - else { - System.err.println("enqueued " + message.messageId()); - } + public void enqueue(Message message) throws IllegalArgumentException { + if (isShutDown.get()) { + log.print(ERROR, "Attempt to enqueue a message when shutdown has been called %s.", message); + return; + } + if (!offer(message)) { + fallback.add(message); + } else { + LOGGER.log(Level.FINE, "enqueued " + message.messageId()); } - + } - // FIXME closeable - public void shutdown() { + @Override + public void close() { if (isShutDown.compareAndSet(false, true)) { final long start = System.currentTimeMillis(); // first let's tell the system to stop looperThread.interrupt(); fallback.close(); + resubmit.close(); shutdownAndWait(networkExecutor, "network"); - log.print( - VERBOSE, "Analytics client shut down in %s ms", (System.currentTimeMillis() - start)); + log.print(VERBOSE, "Analytics client shut down in %s ms", (System.currentTimeMillis() - start)); } } - private void shutdownAndWait(ExecutorService executor, String name) { + private void shutdownAndWait(ExecutorService executor, String name) { try { executor.shutdown(); final boolean executorTerminated = executor.awaitTermination(1, TimeUnit.SECONDS); - log.print( - VERBOSE, - "%s executor %s.", - name, - executorTerminated ? "terminated normally" : "timed out"); + log.print(VERBOSE, "%s executor %s.", name, executorTerminated ? "terminated normally" : "timed out"); } catch (InterruptedException e) { log.print(ERROR, e, "Interrupted while stopping %s executor.", name); Thread.currentThread().interrupt(); @@ -205,8 +160,8 @@ private void shutdownAndWait(ExecutorService executor, String name) { } /** - * Looper runs on a background thread and takes messages from the queue. Once it collects enough - * messages, it triggers a flush. + * Looper runs on a background thread and takes messages from the queue. Once it + * collects enough messages, it triggers a flush. */ class Looper implements Runnable { @@ -219,149 +174,187 @@ public void run() { int currentBatchSize = 0; boolean batchSizeLimitReached = false; int contextSize = gsonInstance.toJson(CONTEXT).getBytes(ENCODING).length; + + long reportedAt = System.currentTimeMillis(); try { - while (!Thread.currentThread().isInterrupted()) { - Message message = messageQueue.poll(flushIntervalInMillis, TimeUnit.MILLISECONDS); - - if (message != null) { - // we do +1 because we are accounting for this new message we just took from the queue - // which is not in list yet - // need to check if this message is going to make us go over the limit considering - // default batch size as well - int defaultBatchSize = - BatchUtility.getBatchDefaultSize(contextSize, messages.size() + 1); - int msgSize = messageSizeInBytes(message); - if (currentBatchSize + msgSize + defaultBatchSize <= BATCH_MAX_SIZE) { - messages.add(message); - currentBatchSize+=msgSize; - } else { - // put message that did not make the cut this time back on the queue, we already took - // this message if we dont put it back its lost - // we take care of that after submitting the batch - batchSizeLimitReached = true; - } - } - - if (messages.isEmpty()) { - continue; - } - - Boolean isBlockingSignal = message == null; - Boolean isOverflow = messages.size() >= flushQueueSize; - - if (!messages.isEmpty() && (isOverflow || isBlockingSignal || batchSizeLimitReached)) { - Batch batch = Batch.create(CONTEXT, new ArrayList<>(messages), writeKey); - log.print( - VERBOSE, - "Batching %s message(s) into batch %s.", - batch.batch().size(), - batch.sequence()); - - Call call = service.upload(uploadUrl, batch); - FailsafeCall failsafeCall = - FailsafeCall.with(failsafe).compose(call); - failsafeCall.executeAsync() - .thenAccept(r -> { - if(is5xx(r.code()) || r.code() == 429) { - handleError(batch, null); - } - }) - .exceptionally(t -> { - handleError(batch, t); - return null; - }); - - currentBatchSize = 0; - messages.clear(); - if (batchSizeLimitReached) { - // If this is true that means the last message that would make us go over the limit - // was not added, - // add it to the now cleared messages list so its not lost - messages.add(message); - } - batchSizeLimitReached = false; - } - } + while (!Thread.currentThread().isInterrupted()) { + Message message = messageQueue.poll(config.flushIntervalInMillis, TimeUnit.MILLISECONDS); + + if (message != null) { + // we do +1 because we are accounting for this new message we just took from the + // queue + // which is not in list yet + // need to check if this message is going to make us go over the limit + // considering + // default batch size as well + int defaultBatchSize = BatchUtility.getBatchDefaultSize(contextSize, messages.size() + 1); + int msgSize = messageSizeInBytes(message); + if (currentBatchSize + msgSize + defaultBatchSize <= BATCH_MAX_SIZE) { + messages.add(message); + currentBatchSize += msgSize; + } else { + // put message that did not make the cut this time back on the queue, we already + // took + // this message if we dont put it back its lost + // we take care of that after submitting the batch + batchSizeLimitReached = true; + } + } + + if (messages.isEmpty()) { + continue; + } + + Boolean isBlockingSignal = message == null; + Boolean isOverflow = messages.size() >= config.flushQueueSize; + + if (!messages.isEmpty() && (isOverflow || isBlockingSignal || batchSizeLimitReached)) { + Batch batch = Batch.create(CONTEXT, new ArrayList<>(messages), writeKey); + log.print(VERBOSE, "Batching %s message(s) into batch %s.", batch.batch().size(), batch.sequence()); + + networkExecutor.submit(new BatchUploadTask(breaker, service, batch, uploadUrl, fallback)); + + currentBatchSize = 0; + messages.clear(); + if (batchSizeLimitReached) { + // If this is true that means the last message that would make us go over the + // limit + // was not added, + // add it to the now cleared messages list so its not lost + messages.add(message); + } + batchSizeLimitReached = false; + } + + long now = System.currentTimeMillis(); + if (now - reportedAt > 2_000) { + LOGGER.log(Level.FINE, "HTTPQueue: " + messageQueue.size()); + if (networkExecutor instanceof ThreadPoolExecutor) { + ThreadPoolExecutor tpe = (ThreadPoolExecutor) networkExecutor; + LOGGER.log(Level.FINE, + String.format("HTTPPool active:%d", tpe.getActiveCount())); + } + reportedAt = now; + } + } } catch (InterruptedException e) { - log.print(DEBUG, "Looper interrupted while polling for messages."); - // XXX CANCEL UPLOAD - } catch (Exception e) { - e.printStackTrace(); - } - // SEND pending + log.print(DEBUG, "Looper interrupted while polling for messages."); + Thread.currentThread().interrupt(); + } + + isShutDown.compareAndSet(false, true); + + Message msg = messageQueue.poll(); + while (msg != null) { + fallback.add(msg); + msg = messageQueue.poll(); + } + log.print(VERBOSE, "Looper stopped"); } - } - - void handleError(Batch batch, Throwable t) { - if(t instanceof CompletionException ) { - if(t.getCause() instanceof CircuitBreakerOpenException) { - System.err.println("OPEN"); - } + + private static boolean is5xx(int status) { + return status >= 500 && status < 600; + } + + static interface SupplierWithException { + T get() throws Exception; + } + + private static boolean upload(final CircuitBreaker breaker, + SupplierWithException> uploadRequest) { + if (breaker.tryAcquirePermit()) { + try { + Response upload = uploadRequest.get(); + if (upload.isSuccessful()) { + breaker.recordSuccess(); + // FIXME handle response ? do not retry those ? + // upload.body().success()) + return true; + } else if (upload.code() == 429) { + breaker.open(); + } else { + breaker.recordFailure(); + } + } catch (Exception e) { + breaker.recordException(e); } - for(Message msg : batch.batch()) { - fallback.add(msg); + } + return false; + } + + static class BatchUploadTask implements Runnable { + final CircuitBreaker breaker; + final SegmentService service; + final Batch batch; + final HttpUrl uploadUrl; + final FallbackAppender fallback; + + BatchUploadTask(final CircuitBreaker breaker, final SegmentService service, final Batch batch, + final HttpUrl uploadUrl, FallbackAppender fallback) { + this.breaker = breaker; + this.service = service; + this.batch = batch; + this.uploadUrl = uploadUrl; + this.fallback = fallback; + } + + @Override + public void run() { + if (!upload(breaker, () -> service.upload(uploadUrl, batch).execute())) { + fallback.add(batch); } + } } - void handleError(Message msg) { - fallback.add(msg); + static class BatchUploadFileTask implements Runnable { + final CircuitBreaker breaker; + final SegmentService service; + final Path path; + final Gson gson; + final HttpUrl uploadUrl; + + static final MediaType JSON = MediaType.get("application/json"); + + BatchUploadFileTask(final CircuitBreaker breaker, final SegmentService service, final Path path, Gson gson, + final HttpUrl uploadUrl) { + this.breaker = breaker; + this.service = service; + this.path = path; + this.gson = gson; + this.uploadUrl = uploadUrl; + } + + @Override + public void run() { + if (upload(breaker, + () -> service.upload(uploadUrl, RequestBody.create(path.toFile(), JSON)).execute())) { + try { + Files.delete(path); + } catch (IOException e) { + // will attempt to submit again (rename file?) + LOGGER.log(Level.WARNING, "Cannot delete file " + path, e); + } + } + } } - private static boolean is5xx(int status) { - return status >= 500 && status < 600; + public void resubmit(Path path) { + networkExecutor.submit(new BatchUploadFileTask(breaker, service, path, gsonInstance, uploadUrl)); } + public static class BatchUtility { - /** - * Method to determine what is the expected default size of the batch regardless of messages - * - *

Sample batch: - * {"batch":[{"type":"alias","messageId":"fc9198f9-d827-47fb-96c8-095bd3405d93","timestamp":"Nov - * 18, 2021, 2:45:07 - * PM","userId":"jorgen25","integrations":{"someKey":{"data":"aaaaa"}},"previousId":"foo"},{"type":"alias", - * "messageId":"3ce6f88c-36cb-4991-83f8-157e10261a89","timestamp":"Nov 18, 2021, 2:45:07 - * PM","userId":"jorgen25", - * "integrations":{"someKey":{"data":"aaaaa"}},"previousId":"foo"},{"type":"alias", - * "messageId":"a328d339-899a-4a14-9835-ec91e303ac4d","timestamp":"Nov 18, 2021, 2:45:07 PM", - * "userId":"jorgen25","integrations":{"someKey":{"data":"aaaaa"}},"previousId":"foo"},{"type":"alias", - * "messageId":"57b0ceb4-a1cf-4599-9fba-0a44c7041004","timestamp":"Nov 18, 2021, 2:45:07 PM", - * "userId":"jorgen25","integrations":{"someKey":{"data":"aaaaa"}},"previousId":"foo"}], - * "sentAt":"Nov 18, 2021, 2:45:07 PM","context":{"library":{"name":"analytics-java", - * "version":"3.1.3"}},"sequence":1,"writeKey":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"} - * - *

total size of batch : 932 - * - *

BREAKDOWN: {"batch":[MESSAGE1,MESSAGE2,MESSAGE3,MESSAGE4],"sentAt":"MMM dd, yyyy, HH:mm:ss - * tt","context":CONTEXT,"sequence":1,"writeKey":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"} - * - *

so we need to account for: 1 -message size: 189 * 4 = 756 2 -context object size = 55 in - * this sample -> 756 + 55 = 811 3 -Metadata (This has the sent data/sequence characters) + - * extra chars (these are chars like "batch":[] or "context": etc and will be pretty much the - * same length in every batch -> size is 73 --> 811 + 73 = 884 (well 72 actually, char 73 is the - * sequence digit which we account for in point 5) 4 -Commas between each message, the total - * number of commas is number_of_msgs - 1 = 3 -> 884 + 3 = 887 (sample is 886 because the hour - * in sentData this time happens to be 2:45 but it could be 12:45 5 -Sequence Number increments - * with every batch created - * - *

so formulae to determine the expected default size of the batch is - * - * @return: defaultSize = messages size + context size + metadata size + comma number + sequence - * digits + writekey + buffer - * @return - */ private static int getBatchDefaultSize(int contextSize, int currentMessageNumber) { - // sample data: {"batch":[],"sentAt":"MMM dd, yyyy, HH:mm:ss tt","context":,"sequence":1, - // "writeKey":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"} - 119 + // sample data: {"batch":[],"sentAt":"MMM dd, yyyy, HH:mm:ss + // tt","context":,"sequence":1, + // "writeKey":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"} - 119 // Don't need to squeeze everything possible into a batch, adding a buffer int metadataExtraCharsSize = 119 + 1024; int commaNumber = currentMessageNumber - 1; - return contextSize - + metadataExtraCharsSize - + commaNumber - + String.valueOf(Integer.MAX_VALUE).length(); + return contextSize + metadataExtraCharsSize + commaNumber + String.valueOf(Integer.MAX_VALUE).length(); } } } diff --git a/analytics/src/main/java/com/segment/analytics/internal/Config.java b/analytics/src/main/java/com/segment/analytics/internal/Config.java new file mode 100644 index 00000000..80d9ceb5 --- /dev/null +++ b/analytics/src/main/java/com/segment/analytics/internal/Config.java @@ -0,0 +1,185 @@ +package com.segment.analytics.internal; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import okhttp3.OkHttpClient; + +public class Config { + + public static final int DEFAULT_FALLBACK_QUEUE_SIZE = 250; + public static final int DEFAULT_FALLBACK_QUEUE_FLUSH_SIZE = 50; + public static final int DEFAULT_FALLBACK_QUEUE_FLUSH_MS = 2_000; + public static final String DEFAULT_FALLBACK_FILE = "pending"; + + // + + public static final int DEFAULT_HTTP_QUEUE_SIZE = Integer.MAX_VALUE; + public static final int DEFAULT_HTTP_QUEUE_FLUSH = 250; + public static final int DEFAULT_HTTP_QUEUE_FLUSH_MS = 10 * 1000; + + public static final int DEFAULT_HTTP_CIRCUIT_ERRORS_IN_A_MINUTE = 10; + public static final int DEFAULT_HTTP_CIRCUIT_SECONDS_IN_OPEN = 30; + public static final int DEFAULT_HTTP_CIRCUIT_REQUESTS_TO_CLOSE = 1; + + // + + public static OkHttpClient defaultClient() { + OkHttpClient client = new OkHttpClient.Builder() + .connectTimeout(15, TimeUnit.SECONDS) + .readTimeout(15, TimeUnit.SECONDS) + .writeTimeout(15, TimeUnit.SECONDS) + .build(); + return client; + } + + public static ExecutorService defaultNetworkExecutor() { + return Executors.newSingleThreadExecutor(defaultThreadFactory()); + } + + public static ThreadFactory defaultThreadFactory() { + return new ThreadFactory() { + @Override + public Thread newThread(final Runnable r) { + return new Thread(new Runnable() { + @Override + public void run() { + Thread.currentThread().setPriority(Thread.MIN_PRIORITY); + r.run(); + } + }); + } + }; + } + + public static class HttpConfig { + final int queueSize; + final int flushQueueSize; + final long flushIntervalInMillis; + + final int circuitErrorsInAMinute; + final int circuitSecondsInOpen; + final int circuitRequestToClose; + + private HttpConfig(Builder builder) { + this.queueSize = builder.queueSize; + this.flushQueueSize = builder.flushQueueSize; + this.flushIntervalInMillis = builder.flushIntervalInMillis; + this.circuitErrorsInAMinute = builder.circuitErrorsInAMinute; + this.circuitSecondsInOpen = builder.circuitSecondsInOpen; + this.circuitRequestToClose = builder.circuitRequestToClose; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private int queueSize = DEFAULT_HTTP_QUEUE_SIZE; + private int flushQueueSize = DEFAULT_HTTP_QUEUE_FLUSH; + private long flushIntervalInMillis = DEFAULT_HTTP_QUEUE_FLUSH_MS; + + private int circuitErrorsInAMinute = DEFAULT_HTTP_CIRCUIT_ERRORS_IN_A_MINUTE; + private int circuitSecondsInOpen = DEFAULT_HTTP_CIRCUIT_SECONDS_IN_OPEN; + private int circuitRequestToClose = DEFAULT_HTTP_CIRCUIT_REQUESTS_TO_CLOSE; + + public Builder queueSize(int value) { + if (value <= 0) { + throw new IllegalArgumentException("queueSize should be positive."); + } + this.queueSize = value; + return this; + } + + public Builder flushQueueSize(int value) { + if (value < 1) { + throw new IllegalArgumentException("flushQueueSize must not be less than 1."); + } + this.flushQueueSize = value; + return this; + } + + public Builder flushIntervalInMillis(long value) { + if (value < 1000) { + throw new IllegalArgumentException("flushIntervalInMillis must not be less than 1 second."); + } + + this.flushIntervalInMillis = value; + return this; + } + + public Builder circuitErrorsInAMinute(int value) { + this.circuitErrorsInAMinute = value; + return this; + } + + public Builder circuitSecondsInOpen(int value) { + this.circuitSecondsInOpen = value; + return this; + } + + public Builder circuitRequestToClose(int value) { + this.circuitRequestToClose = value; + return this; + } + + public HttpConfig build() { + return new HttpConfig(this); + } + } + } + + public static class FileConfig { + /** size of the queue waiting to be written to file */ + final int size; + /** batch size to flush messages to file*/ + final int flushSize; + /** max milliseconds without a flush messages to file*/ + final int flushMs; + /** path to save pending messages */ + final String filePath; + + private FileConfig(Builder builder) { + this.size = builder.size; + this.flushSize = builder.flushSize; + this.flushMs = builder.flushMs; + this.filePath = builder.filePath; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private int size = DEFAULT_FALLBACK_QUEUE_SIZE; + private int flushSize = DEFAULT_FALLBACK_QUEUE_FLUSH_SIZE; + private int flushMs = DEFAULT_FALLBACK_QUEUE_FLUSH_MS; + private String filePath = DEFAULT_FALLBACK_FILE; + + public Builder size(int value) { + this.size = value; + return this; + } + + public Builder flushSize(int value) { + this.flushSize = value; + return this; + } + + public Builder flushMs(int value) { + this.flushMs = value; + return this; + } + + public Builder filePath(String value) { + this.filePath = value; + return this; + } + + public FileConfig build() { + return new FileConfig(this); + } + } + } +} diff --git a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java index 1f85e392..120f51f9 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java +++ b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java @@ -1,225 +1,237 @@ package com.segment.analytics.internal; import com.google.gson.Gson; -import com.google.gson.GsonBuilder; -import com.segment.analytics.gson.AutoValueAdapterFactory; -import com.segment.analytics.gson.ISO8601DateAdapter; +import com.segment.analytics.internal.Config.FileConfig; +import com.segment.analytics.messages.Batch; import com.segment.analytics.messages.Message; -import com.segment.analytics.messages.TrackMessage; -import java.io.File; +import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; -import java.util.Date; import java.util.List; -import java.util.Objects; +import java.util.UUID; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; -import java.util.stream.Collectors; -import org.apache.commons.io.FileSystem; - -public class FallbackAppender { - - private static final int FLUSH_MS = 100; - private static final int BATCH = 20; - private static final int LASTMESSAGE_RETRY_MS = 10_000; - private static final String PATH = "pending"; - - private final AnalyticsClient client; - private final BlockingQueue queue; - private final File file; - private final Lock lock = new ReentrantLock(); - private final Thread writer; - private final Thread reader; - private final Gson gson; - - private transient long lastMessage; - - public FallbackAppender(AnalyticsClient client) { - this.client = client; - this.file = new File(PATH); - this.queue = new ArrayBlockingQueue(100); - this.writer = new Thread(new FileWriter()); // XXX threadFactory daemon - this.reader = new Thread(new FileReader()); // XXX threadFactory daemon - this.gson = new GsonBuilder() - .registerTypeAdapterFactory(new AutoValueAdapterFactory()) - .registerTypeAdapter(Date.class, new ISO8601DateAdapter()) - .create(); - - file.delete(); // FIXME do not remove on start - - this.lastMessage = System.currentTimeMillis(); - this.writer.start(); - this.reader.start(); +import java.util.logging.Level; +import java.util.logging.Logger; + +public class FallbackAppender implements Closeable { + + private static final Logger LOGGER = Logger.getLogger(FallbackAppender.class.getName()); + + public static final String TMP_EXTENSION = ".tmp"; + + /** + * Our servers only accept batches < 500KB. This limit is 475KB to account for + * extra data that is not present in payloads themselves, but is added later, + * such as `sentAt`, `integrations` and other json tokens. + */ + private static final long MAX_BATCH_SIZE = 475_000; // 475KB. + + private Path currentFile; + private Instant currentStart; + private final Path directory; + private final Gson gson; + private final FileConfig config; + private final BlockingQueue queue; + private final Thread writer; + + /** + * @throws IOException if cannot create the configured filePath directory + */ + public FallbackAppender(Gson gson, ThreadFactory threadFactory, FileConfig config) throws IOException { + this.gson = gson; + this.config = config; + this.directory = Files.createDirectories(Path.of(config.filePath)); + + rollover(); + + this.queue = new ArrayBlockingQueue(config.size); + this.writer = threadFactory.newThread(new FileWriter()); + this.writer.setName(FallbackAppender.class.getSimpleName() + "-Writer"); + this.writer.start(); + } + + /** Ends the currentFile and start a new one */ + private void rollover() { + String fileName; + if (currentFile != null) { + fileName = currentFile.getFileName().toString(); + try { + Files.move(currentFile, + currentFile.resolveSibling(fileName.substring(0, fileName.length() - TMP_EXTENSION.length())), + StandardCopyOption.ATOMIC_MOVE); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot rollover " + fileName, e); + } } - public void close() { - reader.interrupt(); - writer.interrupt(); - } - - // block !!! - public void add(Message msg) { - try { - System.err.println("failed " + msg.messageId()); - queue.put(msg); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - - class FileReader implements Runnable { - @Override - public void run() { - while (!Thread.currentThread().isInterrupted()) { - if (queue.isEmpty() && System.currentTimeMillis() - lastMessage > LASTMESSAGE_RETRY_MS) { - if (file.length() == 0) { - continue; - } - - List msgs; - try { - msgs = truncate(20); // XXX messageSize - if (msgs.isEmpty()) { - continue; - } - } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - lastMessage = System.currentTimeMillis(); - continue; - } - - while (!msgs.isEmpty()) { - boolean canEnqueue = true; - for (int i = msgs.size() - 1; canEnqueue && i >= 0; i--) { - Message msg = msgs.get(i); - canEnqueue = client.offer(msg); - if (canEnqueue) { - msgs.remove(i); - System.err.println("reenqueued " + msg.messageId()); - } else { - // slow down next iteration when http queue overflow - try { - Thread.sleep(1_000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } - } - } - - try { - Thread.sleep(1_000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } - } - - class FileWriter implements Runnable { - @Override - public void run() { - final List batch = new ArrayList<>(); - while (!Thread.currentThread().isInterrupted()) { - try { - final Message msg = queue.poll(FLUSH_MS, TimeUnit.MILLISECONDS); - if (msg == null) { - if (!batch.isEmpty()) { - write(batch); - } - } else { - batch.add(msg); - if (batch.size() >= BATCH) { - write(batch); - } - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - if (!batch.isEmpty()) { - write(batch); - } - } - } + currentStart = Instant.now(); + fileName = String.format("%s-%s%s", currentStart.toEpochMilli(), UUID.randomUUID(), TMP_EXTENSION); + this.currentFile = directory.resolve(fileName); + } - List truncate(int numMessages) throws IOException { - lock.lock(); - - if (!file.exists()) { - lock.unlock(); - return Collections.emptyList(); - } - - try (ReversedLinesFileReader reader = ReversedLinesFileReader.builder() - .setPath(file.toPath()) - .setBufferSize(FileSystem.getCurrent().getBlockSize()) - .setCharset(StandardCharsets.UTF_8) - .get()) { - - return reader.readLines(numMessages).stream() - .map(this::fromJson) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - } finally { - lock.unlock(); - } - } - - private static final byte[] NEW_LINE = System.lineSeparator().getBytes(StandardCharsets.UTF_8); + @Override + public void close() { + writer.interrupt(); + } - private void write(List batch) { - lock.lock(); - try (FileChannel fileChannel = FileChannel.open( - file.toPath(), StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE); - OutputStream os = Channels.newOutputStream(fileChannel); - FileLock fileLock = fileChannel.lock(); ) { + /** Write a new file with the content of a batch */ + public void add(Batch batch) { + String fileName = String.format("%s-%s", batch.sentAt().getTime(), UUID.randomUUID()); + Path path = directory.resolve(fileName + TMP_EXTENSION); - for (Message msg : batch) { - os.write(toJson(msg).getBytes(StandardCharsets.UTF_8)); - os.write(NEW_LINE); - } + try ( + FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.APPEND, + StandardOpenOption.CREATE_NEW); + OutputStream os = Channels.newOutputStream(fileChannel)) { - fileChannel.force(true); + os.write(toJson(batch).getBytes(StandardCharsets.UTF_8)); - batch.clear(); + fileChannel.force(true); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot write file batch file " + fileName, e); + } - lastMessage = System.currentTimeMillis(); - } catch (IOException e) { - e.printStackTrace(); // FIXME - } finally { - lock.unlock(); - } + try { + Files.move(path, path.resolveSibling(fileName), StandardCopyOption.ATOMIC_MOVE); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot move file batch file " + fileName, e); + } + } + + /** + * Add elements to be persisted. Called on httpQueue overflow + *

+ * This operation may block the calling thread + *

+ */ + public void add(Message msg) { + try { + LOGGER.log(Level.FINEST, "adding to fallback " + msg.messageId()); + queue.put(msg); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + class FileWriter implements Runnable { + @Override + public void run() { + final List batch = new ArrayList<>(config.flushSize); + while (!Thread.currentThread().isInterrupted()) { + try { + + if (Duration.between(currentStart, Instant.now()).getSeconds() > 60 && currentFile.toFile().exists()) { + endCurrentFile(); + rollover(); + } + + final Message msg = queue.poll(config.flushMs, TimeUnit.MILLISECONDS); + if (msg == null) { + if (!batch.isEmpty()) { + write(batch); + } + } else { + batch.add(msg); + if (batch.size() >= config.flushSize) { + write(batch); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + if (!batch.isEmpty()) { + write(batch); + } + } + } + + private static final byte[] BATCH_BEGIN = "{\"batch\":[".getBytes(StandardCharsets.UTF_8); + private static final byte[] COMMA = ",".getBytes(StandardCharsets.UTF_8); + private static final byte[] BATCH_END = "],\"sentAt\":\"2023-04-19T04:03:46.880Z\",\"writeKey\":\"mywrite\"}" + .getBytes(StandardCharsets.UTF_8); + // FIXME DateTimeUtils + // FIXME mywrite + + private void write(List batch) { + List remaining = writeInternal(batch); + if (!remaining.isEmpty()) { + rollover(); + writeInternal(remaining); } - private String toJson(final Message msg) { - try { - return gson.toJson(msg); - } catch (Exception e) { - e.printStackTrace(); - return null; - } + batch.clear(); + } + + /** @return messages that do not fit in the current file */ + private List writeInternal(List batch) { + try ( + FileChannel fileChannel = FileChannel.open(currentFile, StandardOpenOption.WRITE, StandardOpenOption.APPEND, + StandardOpenOption.CREATE); + OutputStream os = Channels.newOutputStream(fileChannel); + FileLock fileLock = fileChannel.lock();) { + + long currentFileSize = fileChannel.size(); + if (currentFileSize == 0) { + os.write(BATCH_BEGIN); + } + + for (int i = 0; i < batch.size(); i++) { + Message msg = batch.get(i); + byte[] msgBytes = toJson(msg).getBytes(StandardCharsets.UTF_8); + if (msgBytes.length + currentFileSize + COMMA.length + BATCH_END.length > MAX_BATCH_SIZE) { + os.write(BATCH_END); + fileChannel.force(true); + + return batch.subList(i, batch.size()); + } + + os.write(msgBytes); + os.write(COMMA); + } + + fileChannel.force(true); + + return Collections.emptyList(); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "write file " + currentFile, e); + return Collections.emptyList(); + } + } + + private void endCurrentFile() { + try ( + FileChannel fileChannel = FileChannel.open(currentFile, StandardOpenOption.WRITE, StandardOpenOption.APPEND, + StandardOpenOption.CREATE); + OutputStream os = Channels.newOutputStream(fileChannel); + FileLock fileLock = fileChannel.lock();) { + os.write(BATCH_END); + fileChannel.force(true); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "write file " + currentFile, e); } + } - private Message fromJson(final String msg) { - try { - // FIXME only track - return gson.fromJson(msg, TrackMessage.class); - } catch (Exception e) { - e.printStackTrace(); - return null; - } + private String toJson(final Object msg) { + try { + return gson.toJson(msg); + } catch (Exception e) { + throw new RuntimeException(e); } + } } diff --git a/analytics/src/main/java/com/segment/analytics/internal/ResubmitCheck.java b/analytics/src/main/java/com/segment/analytics/internal/ResubmitCheck.java new file mode 100644 index 00000000..f1fe6485 --- /dev/null +++ b/analytics/src/main/java/com/segment/analytics/internal/ResubmitCheck.java @@ -0,0 +1,60 @@ +package com.segment.analytics.internal; + +import com.segment.analytics.internal.Config.FileConfig; +import java.io.Closeable; +import java.io.IOException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Iterator; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; + +// TODO this class, somehow, should keep track of the number of retries. All the filenames start with the creation time +public class ResubmitCheck implements Closeable, Runnable { + private static final Logger LOGGER = Logger.getLogger(ResubmitCheck.class.getName()); + + private final Path directory; + private final AnalyticsClient client; + private final ScheduledExecutorService executor; + + public ResubmitCheck(ThreadFactory threadFactory, FileConfig config, AnalyticsClient client) { + this.directory = Path.of(config.filePath); + this.client = client; + this.executor = Executors.newSingleThreadScheduledExecutor(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = threadFactory.newThread(r); + t.setName(ResubmitCheck.class.getSimpleName()); + return t; + } + }); + this.executor.scheduleWithFixedDelay(this, 0, 1, TimeUnit.MINUTES); + } + + @Override + public void close() { + executor.shutdownNow(); + } + + @Override + public void run() { + try (DirectoryStream files = Files.newDirectoryStream(directory, entry -> Files.isRegularFile(entry) + && !entry.getFileName().toString().endsWith(FallbackAppender.TMP_EXTENSION))) { + Iterator fileIterator = files.iterator(); + while (fileIterator.hasNext()) { + Path file = fileIterator.next(); + + // FIXME filter by instance + client.resubmit(file); + // FIXME use calling thread policy executor + } + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot list directory " + directory, e); + } + } +} diff --git a/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java b/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java deleted file mode 100644 index 09a7a773..00000000 --- a/analytics/src/main/java/com/segment/analytics/internal/ReversedLinesFileReader.java +++ /dev/null @@ -1,552 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.segment.analytics.internal; - -import java.io.Closeable; -import java.io.File; -import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.nio.ByteBuffer; -import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; -import java.nio.charset.Charset; -import java.nio.charset.CharsetEncoder; -import java.nio.charset.StandardCharsets; -import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import org.apache.commons.io.Charsets; -import org.apache.commons.io.FileSystem; -import org.apache.commons.io.StandardLineSeparator; -import org.apache.commons.io.build.AbstractStreamBuilder; - -/** - * Reads lines in a file reversely (similar to a BufferedReader, but starting at the last line). Useful for e.g. searching in log files. - *

- * To build an instance, use {@link Builder}. - *

- * - * @see Builder - * @since 2.2 - */ -public class ReversedLinesFileReader implements Closeable { - - // @formatter:off - /** - * Builds a new {@link ReversedLinesFileReader}. - * - *

- * For example: - *

- * 
{@code
-     * ReversedLinesFileReader r = ReversedLinesFileReader.builder()
-     *   .setPath(path)
-     *   .setBufferSize(4096)
-     *   .setCharset(StandardCharsets.UTF_8)
-     *   .get();}
-     *
- * - * @see #get() - * @since 2.12.0 - */ - // @formatter:on - public static class Builder extends AbstractStreamBuilder { - - /** - * Constructs a new {@link Builder}. - */ - public Builder() { - setBufferSizeDefault(DEFAULT_BLOCK_SIZE); - setBufferSize(DEFAULT_BLOCK_SIZE); - } - - /** - * Builds a new {@link ReversedLinesFileReader}. - *

- * You must set input that supports {@link #getInputStream()} on this builder, otherwise, this method throws an exception. - *

- *

- * This builder use the following aspects: - *

- *
    - *
  • {@link #getInputStream()}
    • - *
  • {@link #getBufferSize()}
    • - *
  • {@link #getCharset()}
    • - *
- * - * @return a new instance. - * @throws IllegalStateException if the {@code origin} is {@code null}. - * @throws UnsupportedOperationException if the origin cannot be converted to a {@link Path}. - * @throws IOException if an I/O error occurs. - * @see #getPath() - * @see #getBufferSize() - * @see #getCharset() - */ - @Override - public ReversedLinesFileReader get() throws IOException { - return new ReversedLinesFileReader(getPath(), getBufferSize(), getCharset()); - } - } - - private final class FilePart { - private final long no; - - private final byte[] data; - - private byte[] leftOver; - - private int currentLastBytePos; - - /** - * Constructs a new instance. - * - * @param no the part number - * @param length its length - * @param leftOverOfLastFilePart remainder - * @throws IOException if there is a problem reading the file - */ - private FilePart(final long no, final int length, final byte[] leftOverOfLastFilePart) throws IOException { - this.no = no; - final int dataLength = length + (leftOverOfLastFilePart != null ? leftOverOfLastFilePart.length : 0); - this.data = new byte[dataLength]; - final long off = (no - 1) * blockSize; - - // read data - if (no > 0 /* file not empty */) { - channel.position(off); - final int countRead = channel.read(ByteBuffer.wrap(data, 0, length)); - if (countRead != length) { - throw new IllegalStateException("Count of requested bytes and actually read bytes don't match"); - } - } - // copy left over part into data arr - if (leftOverOfLastFilePart != null) { - System.arraycopy(leftOverOfLastFilePart, 0, data, length, leftOverOfLastFilePart.length); - } - this.currentLastBytePos = data.length - 1; - this.leftOver = null; - } - - /** - * Constructs the buffer containing any leftover bytes. - */ - private void createLeftOver() { - final int lineLengthBytes = currentLastBytePos + 1; - if (lineLengthBytes > 0) { - // create left over for next block - leftOver = Arrays.copyOf(data, lineLengthBytes); - } else { - leftOver = null; - } - currentLastBytePos = -1; - } - - /** - * Finds the new-line sequence and return its length. - * - * @param data buffer to scan - * @param i start offset in buffer - * @return length of newline sequence or 0 if none found - */ - private int getNewLineMatchByteCount(final byte[] data, final int i) { - for (final byte[] newLineSequence : newLineSequences) { - boolean match = true; - for (int j = newLineSequence.length - 1; j >= 0; j--) { - final int k = i + j - (newLineSequence.length - 1); - match &= k >= 0 && data[k] == newLineSequence[j]; - } - if (match) { - return newLineSequence.length; - } - } - return 0; - } - - /** - * Reads a line. - * - * @return the line or null - */ - private String readLine() { // NOPMD Bug in PMD - - String line = null; - int newLineMatchByteCount; - - final boolean isLastFilePart = no == 1; - - int i = currentLastBytePos; - while (i > -1) { - - if (!isLastFilePart && i < avoidNewlineSplitBufferSize) { - // avoidNewlineSplitBuffer: for all except the last file part we - // take a few bytes to the next file part to avoid splitting of newlines - createLeftOver(); - break; // skip last few bytes and leave it to the next file part - } - - // check for newline - if ((newLineMatchByteCount = getNewLineMatchByteCount(data, i)) > 0 /* found newline */) { - final int lineStart = i + 1; - final int lineLengthBytes = currentLastBytePos - lineStart + 1; - - if (lineLengthBytes < 0) { - throw new IllegalStateException("Unexpected negative line length=" + lineLengthBytes); - } - final byte[] lineData = Arrays.copyOfRange(data, lineStart, lineStart + lineLengthBytes); - - line = new String(lineData, charset); - - currentLastBytePos = i - newLineMatchByteCount; - break; // found line - } - - // move cursor - i -= byteDecrement; - - // end of file part handling - if (i < 0) { - createLeftOver(); - break; // end of file part - } - } - - // last file part handling - if (isLastFilePart && leftOver != null) { - // there will be no line break anymore, this is the first line of the file - line = new String(leftOver, charset); - leftOver = null; - } - - return line; - } - - /** - * Handles block rollover - * - * @return the new FilePart or null - * @throws IOException if there was a problem reading the file - */ - private FilePart rollOver() throws IOException { - - if (currentLastBytePos > -1) { - throw new IllegalStateException("Current currentLastCharPos unexpectedly positive... " - + "last readLine() should have returned something! currentLastCharPos=" + currentLastBytePos); - } - - if (no > 1) { - return new FilePart(no - 1, blockSize, leftOver); - } - // NO 1 was the last FilePart, we're finished - if (leftOver != null) { - throw new IllegalStateException("Unexpected leftover of the last block: leftOverOfThisFilePart=" - + new String(leftOver, charset)); - } - return null; - } - } - - private static final String EMPTY_STRING = ""; - - private static final int DEFAULT_BLOCK_SIZE = FileSystem.getCurrent().getBlockSize(); - - /** - * Constructs a new {@link Builder}. - * - * @return a new {@link Builder}. - * @since 2.12.0 - */ - public static Builder builder() { - return new Builder(); - } - - private final int blockSize; - private final Charset charset; - private final FileChannel channel; - private final FileLock fileLock; - private final long totalByteLength; - private final long totalBlockCount; - private final byte[][] newLineSequences; - private final int avoidNewlineSplitBufferSize; - private final int byteDecrement; - private FilePart currentFilePart; - private boolean trailingNewlineOfFileSkipped; - - /** - * Constructs a ReversedLinesFileReader with default block size of 4KB and the - * platform's default encoding. - * - * @param file the file to be read - * @throws IOException if an I/O error occurs. - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final File file) throws IOException { - this(file, DEFAULT_BLOCK_SIZE, Charset.defaultCharset()); - } - - /** - * Constructs a ReversedLinesFileReader with default block size of 4KB and the - * specified encoding. - * - * @param file the file to be read - * @param charset the charset to use, null uses the default Charset. - * @throws IOException if an I/O error occurs. - * @since 2.5 - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final File file, final Charset charset) throws IOException { - this(file.toPath(), charset); - } - - /** - * Constructs a ReversedLinesFileReader with the given block size and encoding. - * - * @param file the file to be read - * @param blockSize size of the internal buffer (for ideal performance this - * should match with the block size of the underlying file - * system). - * @param charset the encoding of the file, null uses the default Charset. - * @throws IOException if an I/O error occurs. - * @since 2.3 - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final File file, final int blockSize, final Charset charset) throws IOException { - this(file.toPath(), blockSize, charset); - } - - /** - * Constructs a ReversedLinesFileReader with the given block size and encoding. - * - * @param file the file to be read - * @param blockSize size of the internal buffer (for ideal performance this - * should match with the block size of the underlying file - * system). - * @param charsetName the encoding of the file, null uses the default Charset. - * @throws IOException if an I/O error occurs - * @throws java.nio.charset.UnsupportedCharsetException if the encoding is not supported - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final File file, final int blockSize, final String charsetName) throws IOException { - this(file.toPath(), blockSize, charsetName); - } - - /** - * Constructs a ReversedLinesFileReader with default block size of 4KB and the - * specified encoding. - * - * @param file the file to be read - * @param charset the charset to use, null uses the default Charset. - * @throws IOException if an I/O error occurs. - * @since 2.7 - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final Path file, final Charset charset) throws IOException { - this(file, DEFAULT_BLOCK_SIZE, charset); - } - - /** - * Constructs a ReversedLinesFileReader with the given block size and encoding. - * - * @param file the file to be read - * @param blockSize size of the internal buffer (for ideal performance this - * should match with the block size of the underlying file - * system). - * @param charset the encoding of the file, null uses the default Charset. - * @throws IOException if an I/O error occurs. - * @since 2.7 - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final Path file, final int blockSize, final Charset charset) throws IOException { - this.blockSize = blockSize; - this.charset = Charsets.toCharset(charset); - - // --- check & prepare encoding --- - final CharsetEncoder charsetEncoder = this.charset.newEncoder(); - final float maxBytesPerChar = charsetEncoder.maxBytesPerChar(); - if (maxBytesPerChar == 1f || this.charset == StandardCharsets.UTF_8) { - // all one byte encodings are no problem - byteDecrement = 1; - } else if (this.charset == Charset.forName("Shift_JIS") - || // Same as for UTF-8 - // http://www.herongyang.com/Unicode/JIS-Shift-JIS-Encoding.html - this.charset == Charset.forName("windows-31j") - || // Windows code page 932 (Japanese) - this.charset == Charset.forName("x-windows-949") - || // Windows code page 949 (Korean) - this.charset == Charset.forName("gbk") - || // Windows code page 936 (Simplified Chinese) - this.charset == Charset.forName("x-windows-950")) { // Windows code page 950 (Traditional Chinese) - byteDecrement = 1; - } else if (this.charset == StandardCharsets.UTF_16BE || this.charset == StandardCharsets.UTF_16LE) { - // UTF-16 new line sequences are not allowed as second tuple of four byte - // sequences, - // however byte order has to be specified - byteDecrement = 2; - } else if (this.charset == StandardCharsets.UTF_16) { - throw new UnsupportedEncodingException( - "For UTF-16, you need to specify the byte order (use UTF-16BE or " + "UTF-16LE)"); - } else { - throw new UnsupportedEncodingException( - "Encoding " + charset + " is not supported yet (feel free to " + "submit a patch)"); - } - - // NOTE: The new line sequences are matched in the order given, so it is - // important that \r\n is BEFORE \n - this.newLineSequences = new byte[][] { - StandardLineSeparator.CRLF.getBytes(this.charset), - StandardLineSeparator.LF.getBytes(this.charset), - StandardLineSeparator.CR.getBytes(this.charset) - }; - - this.avoidNewlineSplitBufferSize = newLineSequences[0].length; - - // Open file - this.channel = FileChannel.open(file, StandardOpenOption.READ, StandardOpenOption.WRITE); - this.fileLock = channel.lock(); - this.totalByteLength = channel.size(); - int lastBlockLength = (int) (this.totalByteLength % blockSize); - if (lastBlockLength > 0) { - this.totalBlockCount = this.totalByteLength / blockSize + 1; - } else { - this.totalBlockCount = this.totalByteLength / blockSize; - if (this.totalByteLength > 0) { - lastBlockLength = blockSize; - } - } - this.currentFilePart = new FilePart(totalBlockCount, lastBlockLength, null); - } - - /** - * Constructs a ReversedLinesFileReader with the given block size and encoding. - * - * @param file the file to be read - * @param blockSize size of the internal buffer (for ideal performance this - * should match with the block size of the underlying file - * system). - * @param charsetName the encoding of the file, null uses the default Charset. - * @throws IOException if an I/O error occurs - * @throws java.nio.charset.UnsupportedCharsetException if the encoding is not supported - * @since 2.7 - * @deprecated Use {@link #builder()}, {@link Builder}, and {@link Builder#get()} - */ - @Deprecated - public ReversedLinesFileReader(final Path file, final int blockSize, final String charsetName) throws IOException { - this(file, blockSize, Charsets.toCharset(charsetName)); - } - - /** - * Closes underlying resources. - * - * @throws IOException if an I/O error occurs. - */ - @Override - public void close() throws IOException { - fileLock.release(); - channel.close(); - } - - /** - * Returns the lines of the file from bottom to top. - * - * @return the next line or null if the start of the file is reached - * @throws IOException if an I/O error occurs. - */ - public String readLine() throws IOException { - - String line = currentFilePart.readLine(); - while (line == null) { - currentFilePart = currentFilePart.rollOver(); - if (currentFilePart == null) { - // no more FileParts: we're done, leave line set to null - break; - } - line = currentFilePart.readLine(); - } - - // aligned behavior with BufferedReader that doesn't return a last, empty line - if (EMPTY_STRING.equals(line) && !trailingNewlineOfFileSkipped) { - trailingNewlineOfFileSkipped = true; - line = readLine(); - } - - return line; - } - - /** - * Returns {@code lineCount} lines of the file from bottom to top. - *

- * If there are less than {@code lineCount} lines in the file, then that's what - * you get. - *

- *

- * Note: You can easily flip the result with {@link Collections#reverse(List)}. - *

- * - * @param lineCount How many lines to read. - * @return A new list - * @throws IOException if an I/O error occurs. - * @since 2.8.0 - */ - public List readLines(final int lineCount) throws IOException { - if (lineCount < 0) { - throw new IllegalArgumentException("lineCount < 0"); - } - final ArrayList arrayList = new ArrayList<>(lineCount); - for (int i = 0; i < lineCount; i++) { - final String line = readLine(); - if (line == null) { - channel.truncate(0); - return arrayList; - } - arrayList.add(line); - } - - long truncateTo = (this.currentFilePart.no - 1) * blockSize; - truncateTo += this.currentFilePart.currentLastBytePos + 1; - channel.truncate(truncateTo); - - channel.force(true); - - return arrayList; - } - - /** - * Returns the last {@code lineCount} lines of the file. - *

- * If there are less than {@code lineCount} lines in the file, then that's what - * you get. - *

- * - * @param lineCount How many lines to read. - * @return A String. - * @throws IOException if an I/O error occurs. - * @since 2.8.0 - */ - public String toString(final int lineCount) throws IOException { - final List lines = readLines(lineCount); - Collections.reverse(lines); - return lines.isEmpty() ? EMPTY_STRING : String.join(System.lineSeparator(), lines) + System.lineSeparator(); - } -} diff --git a/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java b/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java deleted file mode 100644 index e1c282ba..00000000 --- a/analytics/src/test/java/com/segment/analytics/AnalyticsBuilderTest.java +++ /dev/null @@ -1,429 +0,0 @@ -package com.segment.analytics; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; - -import com.google.gson.GsonBuilder; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mockito; - -public class AnalyticsBuilderTest { - Analytics.Builder builder; - - @Before - public void setUp() { - builder = Analytics.builder("foo"); - } - - @Test - public void nullWriteKey() { - try { - builder = Analytics.builder(null); - fail("Should fail for null writeKey"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("writeKey cannot be null or empty."); - } - } - - @Test - public void emptyWriteKey() { - try { - builder = Analytics.builder(""); - fail("Should fail for empty writeKey"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("writeKey cannot be null or empty."); - } - - try { - builder = Analytics.builder(" "); - fail("Should fail for empty writeKey"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("writeKey cannot be null or empty."); - } - } - - @Test - public void nullUserAgent() { - try { - builder.userAgent(null); - fail("Should fail for null userAgent"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("userAgent cannot be null or empty."); - } - } - - @Test - public void emptyUserAgent() { - try { - builder.userAgent(""); - fail("Should fail for empty userAgent"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("userAgent cannot be null or empty."); - } - - try { - builder.userAgent(" "); - fail("Should fail for empty userAgent"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("userAgent cannot be null or empty."); - } - } - - @Test - public void nullClient() { - try { - builder.client(null); - fail("Should fail for null client"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null client"); - } - } - - @Test - public void nullLog() { - try { - builder.log(null); - fail("Should fail for null log"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null log"); - } - } - - @Test - public void invalidRetryCount() { - try { - builder.retries(0); - fail("Should fail for retries less than 1"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("retries must be at least 1"); - } - - try { - builder.retries(-1); - fail("Should fail for retries less than 1"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("retries must be at least 1"); - } - } - - @Test - public void nullTransformer() { - try { - builder.messageTransformer(null); - fail("Should fail for null transformer"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null transformer"); - } - } - - @Test - public void duplicateTransformer() { - MessageTransformer transformer = mock(MessageTransformer.class); - try { - builder.messageTransformer(transformer).messageTransformer(transformer); - fail("Should fail for duplicate transformer"); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("MessageTransformer is already registered."); - } - } - - @Test - public void buildsWithValidTransformer() { - Analytics analytics = builder.messageTransformer(mock(MessageTransformer.class)).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void nullInterceptor() { - try { - builder.messageInterceptor(null); - fail("Should fail for null interceptor"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null interceptor"); - } - } - - @Test - public void duplicateInterceptor() { - MessageInterceptor interceptor = mock(MessageInterceptor.class); - try { - builder.messageInterceptor(interceptor).messageInterceptor(interceptor); - fail("Should fail for duplicate interceptor"); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("MessageInterceptor is already registered."); - } - } - - @Test - public void buildsWithValidInterceptor() { - Analytics analytics = builder.messageInterceptor(mock(MessageInterceptor.class)).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void nullGsonBuilder() { - try { - builder.gsonBuilder(null); - fail("Should fail for null gsonBuilder"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null gsonBuilder"); - } - } - - @Test - public void duplicateGsonBuilder() { - GsonBuilder gsonBuilder = new GsonBuilder(); - try { - builder.gsonBuilder(gsonBuilder).gsonBuilder(gsonBuilder); - fail("Should fail for duplicate gsonBuilder"); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("gsonBuilder is already registered."); - } - } - - @Test - public void buildsWithValidGsonBuilder() { - GsonBuilder gsonBuilder = new GsonBuilder(); - Analytics analytics = builder.gsonBuilder(gsonBuilder).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void invalidFlushQueueSize() { - try { - builder.flushQueueSize(0); - fail("Should fail for non positive flushQueueSize"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("flushQueueSize must not be less than 1."); - } - - try { - builder.flushQueueSize(-1); - fail("Should fail for non positive flushQueueSize"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("flushQueueSize must not be less than 1."); - } - } - - @Test - public void buildsWithValidFlushQueueSize() { - Analytics analytics = builder.flushQueueSize(1).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void invalidFlushInterval() { - try { - builder.flushInterval(-1, TimeUnit.SECONDS); - fail("Should fail for negative flushInterval"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("flushInterval must not be less than 1 second."); - } - - try { - builder.flushInterval(500, TimeUnit.MILLISECONDS); - fail("Should fail for flushInterval < 1 second"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("flushInterval must not be less than 1 second."); - } - - // Exercise a bug where we only checked the number passed without converting to milliseconds - try { - builder.flushInterval(2000, TimeUnit.NANOSECONDS); - fail("Should fail for flushInterval < 1 second"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("flushInterval must not be less than 1 second."); - } - } - - @Test - public void buildsWithValidFlushInterval() { - Analytics analytics = builder.flushInterval(2, TimeUnit.SECONDS).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void nullNetworkExecutor() { - try { - builder.networkExecutor(null); - fail("Should fail for null network executor"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null networkExecutor"); - } - } - - @Test - public void buildsWithValidNetworkExecutor() { - Analytics analytics = builder.networkExecutor(mock(ExecutorService.class)).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void nullEndpoint() { - try { - builder.endpoint(null); - fail("Should fail for null endpoint"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("endpoint cannot be null or empty."); - } - } - - @Test - public void emptyEndpoint() { - try { - builder.endpoint(""); - fail("Should fail for empty endpoint"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("endpoint cannot be null or empty."); - } - - try { - builder.endpoint(" "); - fail("Should fail for empty endpoint"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("endpoint cannot be null or empty."); - } - } - - @Test - public void buildsWithValidEndpoint() { - Analytics analytics = builder.endpoint("https://api.segment.io").build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void buildsCorrectEndpoint() { - builder.endpoint("https://api.segment.io"); - String expectedURL = "https://api.segment.io/v1/import/"; - assertEquals(expectedURL, builder.endpoint.toString()); - } - - @Test - public void buildsWithValidUploadURL() { - Analytics analytics = builder.setUploadURL("https://example.com/v2/batch/").build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void buildsCorrectEndpointWithUploadURL() { - builder.setUploadURL("https://dummy.url/api/v1/segment/").build(); - String expectedURL = "https://dummy.url/api/v1/segment/"; - assertEquals(expectedURL, builder.endpoint.toString()); - } - - @Test - public void shouldPrioritizeUploadURLOverEndpoint() { - builder - .endpoint("this wont be set anyway") - .setUploadURL("https://dummy.url/api/v1/segment/") - .build(); - String expectedURL = "https://dummy.url/api/v1/segment/"; - - assertEquals(expectedURL, builder.uploadURL.toString()); - assertNotEquals("this wont be set anyway", builder.endpoint.toString()); - } - - @Test - public void buildsCorrectURLWithUploadURL() { - builder.setUploadURL("https://example.com/v2/batch/").build(); - String expectedURL = "https://example.com/v2/batch/"; - assertEquals(expectedURL, builder.uploadURL.toString()); - } - - @Test - public void nullHostAndPrefixEndpoint() { - try { - builder.setUploadURL(null); - fail("Should fail for null endpoint"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Upload URL cannot be null or empty."); - } - } - - @Test - public void emptyHostAndPrefixEndpoint() { - try { - builder.setUploadURL(""); - fail("Should fail for empty endpoint"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Upload URL cannot be null or empty."); - } - - try { - builder.setUploadURL(" "); - fail("Should fail for empty endpoint"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Upload URL cannot be null or empty."); - } - } - - @Test - public void nullThreadFactory() { - try { - builder.threadFactory(null); - fail("Should fail for null thread factory"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null threadFactory"); - } - } - - @Test - public void buildsWithThreadFactory() { - Analytics analytics = builder.threadFactory(mock(ThreadFactory.class)).build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void buildsWithForceTlsV1() { - Analytics analytics = builder.forceTlsVersion1().build(); - assertThat(analytics).isNotNull(); - } - - @Test - public void nullPlugin() { - try { - builder.plugin(null); - fail("Should fail for null plugin"); - } catch (NullPointerException e) { - assertThat(e).hasMessage("Null plugin"); - } - } - - @Test - public void pluginCanConfigure() { - Plugin plugin = Mockito.mock(Plugin.class); - builder.plugin(plugin); - verify(plugin).configure(builder); - } - - @Test - public void invalidQueueCapacity() { - try { - builder.queueCapacity(0); - fail("Should fail when queue capacity is 0"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("capacity should be positive."); - } - - try { - builder.queueCapacity(-1); - fail("Should fail when queue capacity is -1"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("capacity should be positive."); - } - } - - @Test - public void buildWithQueueCapacity() { - Analytics analytics = builder.queueCapacity(10).build(); - assertThat(analytics).isNotNull(); - } -} diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index 26474649..8ac238bb 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -9,17 +9,38 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.github.tomakehurst.wiremock.stubbing.ServeEvent; +import com.segment.analytics.internal.AnalyticsClient; +import com.segment.analytics.internal.Config; +import com.segment.analytics.internal.Config.FileConfig; +import com.segment.analytics.internal.Config.HttpConfig; +import com.segment.analytics.internal.FallbackAppender; import com.segment.analytics.messages.TrackMessage; +import java.io.IOException; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.ConsoleHandler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; +import java.util.logging.SimpleFormatter; +import okhttp3.Dispatcher; +import okhttp3.OkHttpClient; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang.RandomStringUtils; import org.awaitility.Awaitility; import org.junit.After; import org.junit.Before; @@ -43,34 +64,104 @@ public class SegmentTest { Analytics analytics; @Before - public void confWireMockAndClient() { + public void confWireMockAndClient() throws IOException { + FileUtils.deleteDirectory(Path.of(Config.DEFAULT_FALLBACK_FILE).toFile()); + stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); + // from SegmentQueue + Integer SEGMENT_FLUSH_QUEUE_SIZE_DEFAULT = 50; + Integer SEGMENT_QUEUE_SIZE_DEFAULT = 250; + Integer DEFAULT_FLUSH_PERIOD_IN_SECONDS = 10; + + // from SegmentQueue getBoundedNetworkExecutor + Integer SEGMENT_EXECUTOR_QUEU_SIZE_DEFAULT = 0; // 5; + Integer EXECUTOR_SIZE = 1; + Integer EXECUTOR_KEEPALIVE_SECONDS = 15; + + ThreadPoolExecutor boundedNetworkExecutor = new ThreadPoolExecutor( + EXECUTOR_SIZE, // corePoolSize + EXECUTOR_SIZE, // maximumPoolSize + EXECUTOR_KEEPALIVE_SECONDS, // keepAliveTime + TimeUnit.SECONDS, + SEGMENT_EXECUTOR_QUEU_SIZE_DEFAULT == 0 + ? new SynchronousQueue<>(true) + : new ArrayBlockingQueue<>(SEGMENT_EXECUTOR_QUEU_SIZE_DEFAULT, true), + // this will cause the HTTP requests to be handled on AnalyticsClient.Looper + // SegmentQueue was discarding oldest tasks // e.getQueue().poll(); e.execute(r); + new CallerRunsPolicy() { + public void rejectedExecution(Runnable r, ThreadPoolExecutor e) { + // System.err.println("==== Pool exhausted ===="); + super.rejectedExecution(r, e); + } + ; + }); + + // segment Platform getDefaultClient + Integer HTTP_TIMEOUT_SECONDS = 15; + OkHttpClient client = new OkHttpClient.Builder() + .connectTimeout(HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .readTimeout(HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) + .writeTimeout(HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) + // addedd bounded executor + .dispatcher(new Dispatcher(boundedNetworkExecutor)) + .build(); + + // Then it always add the user-agent interceptor + // FIXME not forcing tls + analytics = Analytics.builder("write-key") .endpoint(wireMockRule.baseUrl()) - .flushInterval(1, TimeUnit.SECONDS) - .flushQueueSize(20) - .queueCapacity(50) - // http client + .client(client) + .networkExecutor(boundedNetworkExecutor) + .httpConfig(HttpConfig.builder() + .flushQueueSize(SEGMENT_FLUSH_QUEUE_SIZE_DEFAULT) + .queueSize(SEGMENT_QUEUE_SIZE_DEFAULT) + .flushIntervalInMillis(DEFAULT_FLUSH_PERIOD_IN_SECONDS * 1_000) + .build()) + .fileConfig(FileConfig.builder().build()) .build(); } @After public void tearDown() { - analytics.shutdown(); + analytics.close(); + } + + static void configLogger() { + ConsoleHandler console = new ConsoleHandler(); + console.setLevel(Level.ALL); + console.setFormatter(new SimpleFormatter() { + @Override + public String format(LogRecord record) { + return String.format("[%1$tT.%1$tL] %2$s %n", record.getMillis(), record.getMessage()); + } + }); + + Logger l1 = Logger.getLogger(FallbackAppender.class.getName()); + Logger l2 = Logger.getLogger(AnalyticsClient.class.getName()); + l1.setLevel(Level.ALL); + l1.addHandler(console); + l2.setLevel(Level.ALL); + l2.addHandler(console); } @Test public void test() throws Throwable { + configLogger(); + + int requestsPerSecond = 1_000; + int numClients = 10; + int messageContentChars = 100; + + int timeToRun = 60_000 * 2; + int timeToRestore = 60_000 * 1; + + int responseDelay = 300; stubFor(post(urlEqualTo("/v1/import/")) .willReturn( - WireMock.aResponse().withStatus(503).withBody("fail").withUniformRandomDelay(100, 1_000))); - - int requestsPerSecond = 10; - int numClients = 10; - int timeToRun = 90_000; - int timeToRestore = 30_000; + WireMock.aResponse().withStatus(503).withBody("fail").withFixedDelay(responseDelay))); long start = System.currentTimeMillis(); boolean upAgain = false; @@ -86,14 +177,15 @@ public void test() throws Throwable { String msgid = "m" + id.getAndIncrement(); ids.add(msgid); analytics.enqueue( - TrackMessage.builder("my-track").messageId(msgid).userId("userId")); + TrackMessage.builder("my-track").messageId(msgid).userId("userId") + .context(Map.of("content", RandomStringUtils.randomAlphanumeric(messageContentChars)))); }); } - Thread.sleep(1); + if (!upAgain && System.currentTimeMillis() - start > timeToRestore) { upAgain = true; stubFor(post(urlEqualTo("/v1/import/")) - .willReturn(okJson("{\"success\": \"true\"}").withUniformRandomDelay(100, 1_000))); + .willReturn(okJson("{\"success\": \"true\"}").withFixedDelay(responseDelay))); System.err.println("UP AGAIN"); } } @@ -110,7 +202,9 @@ public void test() throws Throwable { private static final ObjectMapper OM = new ObjectMapper(); private boolean sentMessagesEqualsTo(String... msgIds) { - return new HashSet<>(sentMessages()).equals(new HashSet<>(Arrays.asList(msgIds))); + Set sentMessages = sentMessages(); + System.err.println("Confirmed msgs %d / %d ".formatted(sentMessages.size(), msgIds.length)); + return sentMessages().equals(new HashSet<>(Arrays.asList(msgIds))); } private Set sentMessages() { @@ -135,7 +229,6 @@ private Set sentMessages() { messageIds.add(msgs.next().get("messageId").asText()); } } - System.err.println("Confirmed msgs : " + messageIds.size()); return messageIds; } } From eb58c682bc4b424cce1dd659677f128a34ac9488 Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Thu, 10 Apr 2025 20:19:18 +0200 Subject: [PATCH 15/16] fix end of batch in tmp files. Change test execution --- .../analytics/internal/FallbackAppender.java | 9 +++- .../com/segment/analytics/SegmentTest.java | 42 ++++++++++++------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java index 120f51f9..e500bad1 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java +++ b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java @@ -187,7 +187,8 @@ private List writeInternal(List batch) { FileLock fileLock = fileChannel.lock();) { long currentFileSize = fileChannel.size(); - if (currentFileSize == 0) { + boolean first = currentFileSize == 0; + if (first) { os.write(BATCH_BEGIN); } @@ -201,8 +202,12 @@ private List writeInternal(List batch) { return batch.subList(i, batch.size()); } + if (first) { + first = false; + } else { + os.write(COMMA); + } os.write(msgBytes); - os.write(COMMA); } fileChannel.force(true); diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index 8ac238bb..f3dee8a7 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -17,16 +17,16 @@ import com.segment.analytics.messages.TrackMessage; import java.io.IOException; import java.nio.file.Path; -import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; +import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy; @@ -119,7 +119,9 @@ public void rejectedExecution(Runnable r, ThreadPoolExecutor e) { .queueSize(SEGMENT_QUEUE_SIZE_DEFAULT) .flushIntervalInMillis(DEFAULT_FLUSH_PERIOD_IN_SECONDS * 1_000) .build()) - .fileConfig(FileConfig.builder().build()) + .fileConfig(FileConfig.builder() + //.size(10_000).flushSize(300) + .build()) .build(); } @@ -160,16 +162,18 @@ public void test() throws Throwable { int responseDelay = 300; stubFor(post(urlEqualTo("/v1/import/")) - .willReturn( - WireMock.aResponse().withStatus(503).withBody("fail").withFixedDelay(responseDelay))); + .willReturn( + WireMock.aResponse().withStatus(503).withBody("fail").withFixedDelay(responseDelay))); long start = System.currentTimeMillis(); boolean upAgain = false; final AtomicInteger id = new AtomicInteger(0); - List ids = new ArrayList<>(); + Collection ids = new ConcurrentLinkedQueue<>(); RateLimiter rate = RateLimiter.create(requestsPerSecond); - ExecutorService exec = Executors.newWorkStealingPool(numClients); + + ExecutorService exec = new ThreadPoolExecutor(numClients, numClients, 15l, TimeUnit.SECONDS, + new LinkedBlockingDeque<>(10_000), new CallerRunsPolicy()); while (System.currentTimeMillis() - start < timeToRun) { if (rate.tryAcquire()) { @@ -190,13 +194,15 @@ public void test() throws Throwable { } } - Awaitility.await() - .atMost(10, TimeUnit.MINUTES) - .pollInterval(1, TimeUnit.SECONDS) - .until(() -> sentMessagesEqualsTo(ids.toArray(new String[ids.size()]))); + exec.shutdown(); + exec.awaitTermination(10, TimeUnit.MINUTES); + + + Awaitility.await() + .atMost(10, TimeUnit.MINUTES) + .pollInterval(1, TimeUnit.SECONDS) + .until(() -> sentMessagesEqualsTo(ids.toArray(new String[ids.size()]))); - exec.shutdownNow(); - exec.awaitTermination(10, TimeUnit.SECONDS); } private static final ObjectMapper OM = new ObjectMapper(); @@ -204,10 +210,12 @@ public void test() throws Throwable { private boolean sentMessagesEqualsTo(String... msgIds) { Set sentMessages = sentMessages(); System.err.println("Confirmed msgs %d / %d ".formatted(sentMessages.size(), msgIds.length)); - return sentMessages().equals(new HashSet<>(Arrays.asList(msgIds))); + + return sentMessages.equals(new HashSet<>(Arrays.asList(msgIds))); } private Set sentMessages() { + int count = 0; Set messageIds = new HashSet<>(); for (ServeEvent event : wireMockRule.getAllServeEvents()) { if (event.getResponse().getStatus() != 200) { @@ -226,9 +234,13 @@ private Set sentMessages() { } Iterator msgs = batch.elements(); while (msgs.hasNext()) { + count++; messageIds.add(msgs.next().get("messageId").asText()); } } + if (count != messageIds.size()) { + System.err.println(String.format("Duplicates!, count: %d messageIds: %d", count, messageIds.size())); + } return messageIds; } } From 10a5ac6f79a9083d901da5f3dcd912bfd3069d3c Mon Sep 17 00:00:00 2001 From: Albert Puig Date: Mon, 14 Apr 2025 00:01:46 +0200 Subject: [PATCH 16/16] cleanup --- .gitignore | 3 +- analytics/pom.xml | 12 + .../java/com/segment/analytics/Analytics.java | 34 +- .../analytics/internal/AnalyticsClient.java | 98 +++-- .../segment/analytics/internal/Config.java | 98 +++-- .../analytics/internal/FallbackAppender.java | 386 +++++++++--------- .../com/segment/analytics/SegmentTest.java | 252 +++++------- .../src/test/resources/logging.properties | 7 + 8 files changed, 453 insertions(+), 437 deletions(-) create mode 100644 analytics/src/test/resources/logging.properties diff --git a/.gitignore b/.gitignore index ea323992..82f04479 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ # Created by https://www.gitignore.io +analytics/pending ### Maven ### target/ @@ -128,4 +129,4 @@ atlassian-ide-plugin.xml .classpath .project .settings/ -.factorypath \ No newline at end of file +.factorypath diff --git a/analytics/pom.xml b/analytics/pom.xml index 8e61a10a..25d9fe31 100644 --- a/analytics/pom.xml +++ b/analytics/pom.xml @@ -108,6 +108,18 @@ + + org.apache.maven.plugins + maven-surefire-plugin + 3.5.3 + + + + ${project.basedir}/src/test/resources/logging.properties + + + + diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java index 67344ae2..5b3f2b15 100644 --- a/analytics/src/main/java/com/segment/analytics/Analytics.java +++ b/analytics/src/main/java/com/segment/analytics/Analytics.java @@ -131,14 +131,12 @@ public static class Builder { private static final String DEFAULT_USER_AGENT = "analytics-java/" + AnalyticsVersion.get(); private final String writeKey; - private OkHttpClient client; private Log log; public HttpUrl endpoint; public HttpUrl uploadURL; private String userAgent = DEFAULT_USER_AGENT; private List messageTransformers; private List messageInterceptors; - private ExecutorService networkExecutor; private ThreadFactory threadFactory; private boolean forceTlsV1 = false; private GsonBuilder gsonBuilder; @@ -152,15 +150,6 @@ public static class Builder { this.writeKey = writeKey; } - /** Set a custom networking client. */ - public Builder client(OkHttpClient client) { - if (client == null) { - throw new NullPointerException("Null client"); - } - this.client = client; - return this; - } - /** Configure debug logging mechanism. By default, nothing is logged. */ public Builder log(Log log) { if (log == null) { @@ -248,15 +237,6 @@ public Builder gsonBuilder(GsonBuilder gsonBuilder) { return this; } - /** Set the {@link ExecutorService} on which all HTTP requests will be made. */ - public Builder networkExecutor(ExecutorService networkExecutor) { - if (networkExecutor == null) { - throw new NullPointerException("Null networkExecutor"); - } - this.networkExecutor = networkExecutor; - return this; - } - /** Set the {@link ThreadFactory} used to create threads. */ @Beta public Builder threadFactory(ThreadFactory threadFactory) { @@ -329,15 +309,9 @@ public Analytics build() throws IOException { } else { messageInterceptors = Collections.unmodifiableList(messageInterceptors); } - if (networkExecutor == null) { - networkExecutor = Config.defaultNetworkExecutor(); - } if (threadFactory == null) { threadFactory = Config.defaultThreadFactory(); } - if (client == null) { - client = Config.defaultClient(); - } if(httpConfig == null) { httpConfig = HttpConfig.builder().build(); } @@ -357,7 +331,7 @@ public void log(String message) { interceptor.setLevel(HttpLoggingInterceptor.Level.BASIC); OkHttpClient.Builder builder = - client + httpConfig.client .newBuilder() .addInterceptor(new AnalyticsRequestInterceptor(userAgent)) .addInterceptor(interceptor); @@ -372,18 +346,18 @@ public void log(String message) { builder = builder.connectionSpecs(Arrays.asList(connectionSpec)); } - client = builder.build(); + httpConfig.client = builder.build(); Retrofit restAdapter = new Retrofit.Builder() .addConverterFactory(GsonConverterFactory.create(gson)) .baseUrl(DEFAULT_ENDPOINT) - .client(client) + .client(httpConfig.client) .build(); SegmentService segmentService = restAdapter.create(SegmentService.class); - AnalyticsClient analyticsClient = new AnalyticsClient(endpoint, segmentService, log, threadFactory, networkExecutor, writeKey, gson, httpConfig, fileConfig); + AnalyticsClient analyticsClient = new AnalyticsClient(endpoint, segmentService, log, threadFactory, writeKey, gson, httpConfig, fileConfig); return new Analytics(analyticsClient, messageTransformers, messageInterceptors, log); } } diff --git a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java index 2ca27085..2dc872c2 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java +++ b/analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java @@ -74,7 +74,7 @@ public class AnalyticsClient implements Closeable { private final ResubmitCheck resubmit; public AnalyticsClient(HttpUrl uploadUrl, SegmentService service, Log log, ThreadFactory threadFactory, - ExecutorService networkExecutor, String writeKey, Gson gsonInstance, HttpConfig config, FileConfig fileConfig) + String writeKey, Gson gsonInstance, HttpConfig config, FileConfig fileConfig) throws IOException { this.config = config; this.messageQueue = new LinkedBlockingQueue(config.queueSize); @@ -83,7 +83,7 @@ public AnalyticsClient(HttpUrl uploadUrl, SegmentService service, Log log, Threa this.log = log; this.looperThread = threadFactory.newThread(new Looper()); this.looperThread.setName(AnalyticsClient.class.getSimpleName() + "-Looper"); - this.networkExecutor = networkExecutor; + this.networkExecutor = config.executor; this.writeKey = writeKey; this.gsonInstance = gsonInstance; @@ -127,7 +127,7 @@ public void enqueue(Message message) throws IllegalArgumentException { if (!offer(message)) { fallback.add(message); } else { - LOGGER.log(Level.FINE, "enqueued " + message.messageId()); + LOGGER.log(Level.FINE, "enqueued {0}", message.messageId()); } } @@ -212,7 +212,7 @@ public void run() { Batch batch = Batch.create(CONTEXT, new ArrayList<>(messages), writeKey); log.print(VERBOSE, "Batching %s message(s) into batch %s.", batch.batch().size(), batch.sequence()); - networkExecutor.submit(new BatchUploadTask(breaker, service, batch, uploadUrl, fallback)); + networkExecutor.submit(new UploadBatchTask(breaker, service, uploadUrl, batch, fallback)); currentBatchSize = 0; messages.clear(); @@ -228,11 +228,10 @@ public void run() { long now = System.currentTimeMillis(); if (now - reportedAt > 2_000) { - LOGGER.log(Level.FINE, "HTTPQueue: " + messageQueue.size()); + LOGGER.log(Level.FINE, "HTTPQueue: {0}", messageQueue.size()); if (networkExecutor instanceof ThreadPoolExecutor) { ThreadPoolExecutor tpe = (ThreadPoolExecutor) networkExecutor; - LOGGER.log(Level.FINE, - String.format("HTTPPool active:%d", tpe.getActiveCount())); + LOGGER.log(Level.FINE, "HTTPPool active:{0}", tpe.getActiveCount()); } reportedAt = now; } @@ -262,74 +261,73 @@ static interface SupplierWithException { T get() throws Exception; } - private static boolean upload(final CircuitBreaker breaker, - SupplierWithException> uploadRequest) { - if (breaker.tryAcquirePermit()) { - try { - Response upload = uploadRequest.get(); - if (upload.isSuccessful()) { - breaker.recordSuccess(); - // FIXME handle response ? do not retry those ? - // upload.body().success()) - return true; - } else if (upload.code() == 429) { - breaker.open(); - } else { - breaker.recordFailure(); - } - } catch (Exception e) { - breaker.recordException(e); - } + + + static abstract class UploadTask implements Runnable{ + final CircuitBreaker breaker; + final SegmentService service; + final HttpUrl uploadUrl; + public UploadTask(CircuitBreaker breaker, SegmentService service, HttpUrl uploadUrl) { + this.breaker = breaker; + this.service = service; + this.uploadUrl = uploadUrl; } - return false; + + boolean upload(SupplierWithException> uploadRequest) { + if (breaker.tryAcquirePermit()) { + try { + Response upload = uploadRequest.get(); + if (upload.isSuccessful()) { + breaker.recordSuccess(); + // FIXME handle response ? do not retry those ? + // upload.body().success()) + return true; + } else if (upload.code() == 429) { + breaker.open(); + } else { + breaker.recordFailure(); + } + } catch (Exception e) { + breaker.recordException(e); + } + } + return false; + } } + + static class UploadBatchTask extends UploadTask { - static class BatchUploadTask implements Runnable { - final CircuitBreaker breaker; - final SegmentService service; final Batch batch; - final HttpUrl uploadUrl; final FallbackAppender fallback; - BatchUploadTask(final CircuitBreaker breaker, final SegmentService service, final Batch batch, - final HttpUrl uploadUrl, FallbackAppender fallback) { - this.breaker = breaker; - this.service = service; + UploadBatchTask(final CircuitBreaker breaker, final SegmentService service, final HttpUrl uploadUrl, final Batch batch + , FallbackAppender fallback) { + super(breaker, service, uploadUrl); this.batch = batch; - this.uploadUrl = uploadUrl; this.fallback = fallback; } @Override public void run() { - if (!upload(breaker, () -> service.upload(uploadUrl, batch).execute())) { + if (!upload(() -> service.upload(uploadUrl, batch).execute())) { fallback.add(batch); } } } - static class BatchUploadFileTask implements Runnable { - final CircuitBreaker breaker; - final SegmentService service; + static class UploadFileTask extends UploadTask { final Path path; - final Gson gson; - final HttpUrl uploadUrl; static final MediaType JSON = MediaType.get("application/json"); - BatchUploadFileTask(final CircuitBreaker breaker, final SegmentService service, final Path path, Gson gson, - final HttpUrl uploadUrl) { - this.breaker = breaker; - this.service = service; + UploadFileTask(final CircuitBreaker breaker, final SegmentService service, final HttpUrl uploadUrl, final Path path) { + super(breaker, service, uploadUrl); this.path = path; - this.gson = gson; - this.uploadUrl = uploadUrl; } @Override public void run() { - if (upload(breaker, - () -> service.upload(uploadUrl, RequestBody.create(path.toFile(), JSON)).execute())) { + if (upload(() -> service.upload(uploadUrl, RequestBody.create(path.toFile(), JSON)).execute())) { try { Files.delete(path); } catch (IOException e) { @@ -341,7 +339,7 @@ public void run() { } public void resubmit(Path path) { - networkExecutor.submit(new BatchUploadFileTask(breaker, service, path, gsonInstance, uploadUrl)); + networkExecutor.submit(new UploadFileTask(breaker, service, uploadUrl, path)); } public static class BatchUtility { diff --git a/analytics/src/main/java/com/segment/analytics/internal/Config.java b/analytics/src/main/java/com/segment/analytics/internal/Config.java index 80d9ceb5..5f0a5ad0 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/Config.java +++ b/analytics/src/main/java/com/segment/analytics/internal/Config.java @@ -1,42 +1,37 @@ package com.segment.analytics.internal; +import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; +import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy; import java.util.concurrent.TimeUnit; +import okhttp3.Dispatcher; import okhttp3.OkHttpClient; public class Config { - public static final int DEFAULT_FALLBACK_QUEUE_SIZE = 250; - public static final int DEFAULT_FALLBACK_QUEUE_FLUSH_SIZE = 50; - public static final int DEFAULT_FALLBACK_QUEUE_FLUSH_MS = 2_000; - public static final String DEFAULT_FALLBACK_FILE = "pending"; + // from SegmentQueue + public static final int DEFAULT_HTTP_QUEUE_SIZE = 250; // analytics-java Integer.MAX_VALUE; + public static final int DEFAULT_HTTP_QUEUE_FLUSH = 50; // analytics-java 250; + public static final int DEFAULT_HTTP_QUEUE_FLUSH_MS = 10 * 1000; - // + public static final int DEFAULT_HTTP_EXECUTOR_SIZE = 1; + public static final int DEFAULT_HTTP_EXECUTOR_QUEUE_SIZE = 0; // SegmentQueue 5; - public static final int DEFAULT_HTTP_QUEUE_SIZE = Integer.MAX_VALUE; - public static final int DEFAULT_HTTP_QUEUE_FLUSH = 250; - public static final int DEFAULT_HTTP_QUEUE_FLUSH_MS = 10 * 1000; + public static final int DEFAULT_HTTP_TIMEOUT_SECONDS = 15; public static final int DEFAULT_HTTP_CIRCUIT_ERRORS_IN_A_MINUTE = 10; public static final int DEFAULT_HTTP_CIRCUIT_SECONDS_IN_OPEN = 30; public static final int DEFAULT_HTTP_CIRCUIT_REQUESTS_TO_CLOSE = 1; - // - - public static OkHttpClient defaultClient() { - OkHttpClient client = new OkHttpClient.Builder() - .connectTimeout(15, TimeUnit.SECONDS) - .readTimeout(15, TimeUnit.SECONDS) - .writeTimeout(15, TimeUnit.SECONDS) - .build(); - return client; - } - - public static ExecutorService defaultNetworkExecutor() { - return Executors.newSingleThreadExecutor(defaultThreadFactory()); - } + // FALLBACK + public static final int DEFAULT_FALLBACK_QUEUE_SIZE = 250; + public static final int DEFAULT_FALLBACK_QUEUE_FLUSH_SIZE = 50; + public static final int DEFAULT_FALLBACK_QUEUE_FLUSH_MS = 2_000; + public static final int DEFAULT_FALLBACK_ROLLOVER_TIMEOUT_SECONDS = 60; + public static final String DEFAULT_FALLBACK_FILE = "pending"; public static ThreadFactory defaultThreadFactory() { return new ThreadFactory() { @@ -62,6 +57,9 @@ public static class HttpConfig { final int circuitSecondsInOpen; final int circuitRequestToClose; + final ExecutorService executor; + public OkHttpClient client; // Analytics touch the instance + private HttpConfig(Builder builder) { this.queueSize = builder.queueSize; this.flushQueueSize = builder.flushQueueSize; @@ -69,6 +67,32 @@ private HttpConfig(Builder builder) { this.circuitErrorsInAMinute = builder.circuitErrorsInAMinute; this.circuitSecondsInOpen = builder.circuitSecondsInOpen; this.circuitRequestToClose = builder.circuitRequestToClose; + + this.executor = new ThreadPoolExecutor( + builder.executorSize, + builder.executorSize, + 15, + TimeUnit.SECONDS, + builder.executorQueueSize == 0 + ? new SynchronousQueue<>(true) + : new ArrayBlockingQueue<>(builder.executorQueueSize, true), + // this will cause the HTTP requests to be handled on AnalyticsClient.Looper + // SegmentQueue was discarding oldest tasks // e.getQueue().poll(); e.execute(r); + new CallerRunsPolicy() { + public void rejectedExecution(Runnable r, ThreadPoolExecutor e) { + // System.err.println("==== NetowrkPool exhausted, running in %s + // ====".formatted(Thread.currentThread().getName())); + super.rejectedExecution(r, e); + } + }); + + this.client = new OkHttpClient.Builder() + .connectTimeout(builder.timeoutSeconds, TimeUnit.SECONDS) + .readTimeout(builder.timeoutSeconds, TimeUnit.SECONDS) + .writeTimeout(builder.timeoutSeconds, TimeUnit.SECONDS) + // use same executor + .dispatcher(new Dispatcher(this.executor)) + .build(); } public static Builder builder() { @@ -84,6 +108,10 @@ public static class Builder { private int circuitSecondsInOpen = DEFAULT_HTTP_CIRCUIT_SECONDS_IN_OPEN; private int circuitRequestToClose = DEFAULT_HTTP_CIRCUIT_REQUESTS_TO_CLOSE; + private int executorSize = DEFAULT_HTTP_EXECUTOR_SIZE; + private int executorQueueSize = DEFAULT_HTTP_EXECUTOR_QUEUE_SIZE; + private int timeoutSeconds = DEFAULT_HTTP_TIMEOUT_SECONDS; + public Builder queueSize(int value) { if (value <= 0) { throw new IllegalArgumentException("queueSize should be positive."); @@ -124,6 +152,21 @@ public Builder circuitRequestToClose(int value) { return this; } + public Builder executorSize(int value) { + this.executorSize = value; + return this; + } + + public Builder executorQueueSize(int value) { + this.executorQueueSize = value; + return this; + } + + public Builder timeoutSeconds(int value) { + this.timeoutSeconds = value; + return this; + } + public HttpConfig build() { return new HttpConfig(this); } @@ -139,12 +182,15 @@ public static class FileConfig { final int flushMs; /** path to save pending messages */ final String filePath; + /** max time to keep a open overflow file before finish the batch */ + final int rolloverTimeoutSeconds; private FileConfig(Builder builder) { this.size = builder.size; this.flushSize = builder.flushSize; this.flushMs = builder.flushMs; this.filePath = builder.filePath; + this.rolloverTimeoutSeconds = builder.rolloverTimeoutSeconds; } public static Builder builder() { @@ -156,6 +202,7 @@ public static class Builder { private int flushSize = DEFAULT_FALLBACK_QUEUE_FLUSH_SIZE; private int flushMs = DEFAULT_FALLBACK_QUEUE_FLUSH_MS; private String filePath = DEFAULT_FALLBACK_FILE; + private int rolloverTimeoutSeconds = DEFAULT_FALLBACK_ROLLOVER_TIMEOUT_SECONDS; public Builder size(int value) { this.size = value; @@ -177,6 +224,11 @@ public Builder filePath(String value) { return this; } + public Builder rolloverTimeoutSeconds(int value) { + this.rolloverTimeoutSeconds = value; + return this; + } + public FileConfig build() { return new FileConfig(this); } diff --git a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java index e500bad1..202d8fe4 100644 --- a/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java +++ b/analytics/src/main/java/com/segment/analytics/internal/FallbackAppender.java @@ -7,9 +7,9 @@ import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; +import java.io.Writer; import java.nio.channels.Channels; import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -30,213 +30,217 @@ public class FallbackAppender implements Closeable { - private static final Logger LOGGER = Logger.getLogger(FallbackAppender.class.getName()); - - public static final String TMP_EXTENSION = ".tmp"; - - /** - * Our servers only accept batches < 500KB. This limit is 475KB to account for - * extra data that is not present in payloads themselves, but is added later, - * such as `sentAt`, `integrations` and other json tokens. - */ - private static final long MAX_BATCH_SIZE = 475_000; // 475KB. - - private Path currentFile; - private Instant currentStart; - private final Path directory; - private final Gson gson; - private final FileConfig config; - private final BlockingQueue queue; - private final Thread writer; - - /** - * @throws IOException if cannot create the configured filePath directory - */ - public FallbackAppender(Gson gson, ThreadFactory threadFactory, FileConfig config) throws IOException { - this.gson = gson; - this.config = config; - this.directory = Files.createDirectories(Path.of(config.filePath)); - - rollover(); - - this.queue = new ArrayBlockingQueue(config.size); - this.writer = threadFactory.newThread(new FileWriter()); - this.writer.setName(FallbackAppender.class.getSimpleName() + "-Writer"); - this.writer.start(); - } - - /** Ends the currentFile and start a new one */ - private void rollover() { - String fileName; - if (currentFile != null) { - fileName = currentFile.getFileName().toString(); - try { - Files.move(currentFile, - currentFile.resolveSibling(fileName.substring(0, fileName.length() - TMP_EXTENSION.length())), - StandardCopyOption.ATOMIC_MOVE); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "Cannot rollover " + fileName, e); - } + private static final Logger LOGGER = Logger.getLogger(FallbackAppender.class.getName()); + + public static final String TMP_EXTENSION = ".tmp"; + + /** + * Our servers only accept batches < 500KB. This limit is 475KB to account for + * extra data that is not present in payloads themselves, but is added later, + * such as `sentAt`, `integrations` and other json tokens. + */ + private static final long MAX_BATCH_SIZE = 475_000; // 475KB. + + private Path currentFile; + private Instant currentStart; + private final Path directory; + private final Gson gson; + private final FileConfig config; + private final BlockingQueue queue; + private final Thread writer; + + /** + * @throws IOException if cannot create the configured filePath directory + */ + public FallbackAppender(Gson gson, ThreadFactory threadFactory, FileConfig config) throws IOException { + this.gson = gson; + this.config = config; + this.directory = Files.createDirectories(Path.of(config.filePath)); + + rollover(); + + this.queue = new ArrayBlockingQueue(config.size); + this.writer = threadFactory.newThread(new FileWriter()); + this.writer.setName(FallbackAppender.class.getSimpleName()); + this.writer.start(); } - currentStart = Instant.now(); - fileName = String.format("%s-%s%s", currentStart.toEpochMilli(), UUID.randomUUID(), TMP_EXTENSION); - this.currentFile = directory.resolve(fileName); - } + /** Ends the currentFile and start a new one */ + private void rollover() { + String fileName; + if (currentFile != null) { + fileName = currentFile.getFileName().toString(); + try { + Files.move( + currentFile, + currentFile.resolveSibling(fileName.substring(0, fileName.length() - TMP_EXTENSION.length())), + StandardCopyOption.ATOMIC_MOVE); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot rollover " + fileName, e); + } + } + + currentStart = Instant.now(); + fileName = String.format("%s-%s%s", currentStart.toEpochMilli(), UUID.randomUUID(), TMP_EXTENSION); + this.currentFile = directory.resolve(fileName); + LOGGER.log(Level.FINE, "currentFile : {0}", fileName); + } + + @Override + public void close() { + writer.interrupt(); + } - @Override - public void close() { - writer.interrupt(); - } + /** Write a new file with the content of a batch */ + public void add(Batch batch) { + String fileName = String.format("%s-%s", batch.sentAt().getTime(), UUID.randomUUID()); + Path path = directory.resolve(fileName + TMP_EXTENSION); - /** Write a new file with the content of a batch */ - public void add(Batch batch) { - String fileName = String.format("%s-%s", batch.sentAt().getTime(), UUID.randomUUID()); - Path path = directory.resolve(fileName + TMP_EXTENSION); + try (FileChannel fileChannel = FileChannel.open( + path, StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE_NEW); + Writer w = Channels.newWriter(fileChannel, StandardCharsets.UTF_8)) { - try ( - FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.APPEND, - StandardOpenOption.CREATE_NEW); - OutputStream os = Channels.newOutputStream(fileChannel)) { + saveBatch(batch, w); - os.write(toJson(batch).getBytes(StandardCharsets.UTF_8)); + // TODO fileChannel.force(true); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot write file batch file " + fileName, e); + } - fileChannel.force(true); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "Cannot write file batch file " + fileName, e); + try { + Files.move(path, path.resolveSibling(fileName), StandardCopyOption.ATOMIC_MOVE); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Cannot move file batch file " + fileName, e); + } } - try { - Files.move(path, path.resolveSibling(fileName), StandardCopyOption.ATOMIC_MOVE); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "Cannot move file batch file " + fileName, e); + /** + * Add elements to be persisted. Called on httpQueue overflow + *

+ * This operation may block the calling thread + *

+ */ + public void add(Message msg) { + try { + LOGGER.log(Level.FINEST, "adding to fallback {0}", msg.messageId()); + queue.put(msg); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } - } - - /** - * Add elements to be persisted. Called on httpQueue overflow - *

- * This operation may block the calling thread - *

- */ - public void add(Message msg) { - try { - LOGGER.log(Level.FINEST, "adding to fallback " + msg.messageId()); - queue.put(msg); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + + class FileWriter implements Runnable { + @Override + public void run() { + final List batch = new ArrayList<>(config.flushSize); + while (!Thread.currentThread().isInterrupted()) { + try { + + if (Duration.between(currentStart, Instant.now()).getSeconds() > config.rolloverTimeoutSeconds + && currentFile.toFile().exists()) { + endCurrentFile(); + rollover(); + } + + final Message msg = queue.poll(config.flushMs, TimeUnit.MILLISECONDS); + if (msg == null) { + if (!batch.isEmpty()) { + write(batch); + } + } else { + batch.add(msg); + if (batch.size() >= config.flushSize) { + write(batch); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + if (!batch.isEmpty()) { + write(batch); + } + } } - } - class FileWriter implements Runnable { - @Override - public void run() { - final List batch = new ArrayList<>(config.flushSize); - while (!Thread.currentThread().isInterrupted()) { - try { - - if (Duration.between(currentStart, Instant.now()).getSeconds() > 60 && currentFile.toFile().exists()) { - endCurrentFile(); - rollover(); - } - - final Message msg = queue.poll(config.flushMs, TimeUnit.MILLISECONDS); - if (msg == null) { - if (!batch.isEmpty()) { - write(batch); - } - } else { - batch.add(msg); - if (batch.size() >= config.flushSize) { - write(batch); - } - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - if (!batch.isEmpty()) { - write(batch); - } + private static final byte[] BATCH_BEGIN = "{\"batch\":[".getBytes(StandardCharsets.UTF_8); + private static final byte[] COMMA = ",".getBytes(StandardCharsets.UTF_8); + private static final byte[] BATCH_END = + "],\"sentAt\":\"2023-04-19T04:03:46.880Z\",\"writeKey\":\"mywrite\"}".getBytes(StandardCharsets.UTF_8); + // FIXME DateTimeUtils + // FIXME mywrite + + private void write(List batch) { + List remaining = writeInternal(batch); + while (!remaining.isEmpty()) { + rollover(); + remaining = writeInternal(remaining); + } + + batch.clear(); } - } - - private static final byte[] BATCH_BEGIN = "{\"batch\":[".getBytes(StandardCharsets.UTF_8); - private static final byte[] COMMA = ",".getBytes(StandardCharsets.UTF_8); - private static final byte[] BATCH_END = "],\"sentAt\":\"2023-04-19T04:03:46.880Z\",\"writeKey\":\"mywrite\"}" - .getBytes(StandardCharsets.UTF_8); - // FIXME DateTimeUtils - // FIXME mywrite - - private void write(List batch) { - List remaining = writeInternal(batch); - if (!remaining.isEmpty()) { - rollover(); - writeInternal(remaining); + + /** @return messages that do not fit in the current file */ + private List writeInternal(List batch) { + try (FileChannel fileChannel = FileChannel.open( + currentFile, StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE); + OutputStream os = Channels.newOutputStream(fileChannel)) { + + long currentFileSize = fileChannel.size(); + boolean first = currentFileSize == 0; + if (first) { + os.write(BATCH_BEGIN); + } + + for (int i = 0; i < batch.size(); i++) { + Message msg = batch.get(i); + byte[] msgBytes = toJson(msg).getBytes(StandardCharsets.UTF_8); + if (msgBytes.length + currentFileSize + COMMA.length + BATCH_END.length > MAX_BATCH_SIZE) { + os.write(BATCH_END); + // TODO fileChannel.force(true); + + return batch.subList(i, batch.size()); + } + + if (first) { + first = false; + } else { + os.write(COMMA); + } + os.write(msgBytes); + } + // TODO fileChannel.force(true); + return Collections.emptyList(); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "write file " + currentFile, e); + return Collections.emptyList(); + } } - batch.clear(); - } - - /** @return messages that do not fit in the current file */ - private List writeInternal(List batch) { - try ( - FileChannel fileChannel = FileChannel.open(currentFile, StandardOpenOption.WRITE, StandardOpenOption.APPEND, - StandardOpenOption.CREATE); - OutputStream os = Channels.newOutputStream(fileChannel); - FileLock fileLock = fileChannel.lock();) { - - long currentFileSize = fileChannel.size(); - boolean first = currentFileSize == 0; - if (first) { - os.write(BATCH_BEGIN); - } - - for (int i = 0; i < batch.size(); i++) { - Message msg = batch.get(i); - byte[] msgBytes = toJson(msg).getBytes(StandardCharsets.UTF_8); - if (msgBytes.length + currentFileSize + COMMA.length + BATCH_END.length > MAX_BATCH_SIZE) { - os.write(BATCH_END); - fileChannel.force(true); - - return batch.subList(i, batch.size()); - } - - if (first) { - first = false; - } else { - os.write(COMMA); - } - os.write(msgBytes); - } - - fileChannel.force(true); - - return Collections.emptyList(); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "write file " + currentFile, e); - return Collections.emptyList(); + private void endCurrentFile() { + try (FileChannel fileChannel = FileChannel.open( + currentFile, StandardOpenOption.WRITE, StandardOpenOption.APPEND, StandardOpenOption.CREATE); + OutputStream os = Channels.newOutputStream(fileChannel)) { + os.write(BATCH_END); + // TODO fileChannel.force(true); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "write file " + currentFile, e); + } } - } - - private void endCurrentFile() { - try ( - FileChannel fileChannel = FileChannel.open(currentFile, StandardOpenOption.WRITE, StandardOpenOption.APPEND, - StandardOpenOption.CREATE); - OutputStream os = Channels.newOutputStream(fileChannel); - FileLock fileLock = fileChannel.lock();) { - os.write(BATCH_END); - fileChannel.force(true); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "write file " + currentFile, e); + + private String toJson(final Object msg) { + try { + return gson.toJson(msg); + } catch (Exception e) { + throw new RuntimeException(e); + } } - } - private String toJson(final Object msg) { - try { - return gson.toJson(msg); - } catch (Exception e) { - throw new RuntimeException(e); + private void saveBatch(final Batch batch, Writer file) { + try { + gson.toJson(batch, file); + } catch (Exception e) { + throw new RuntimeException(e); + } } - } } diff --git a/analytics/src/test/java/com/segment/analytics/SegmentTest.java b/analytics/src/test/java/com/segment/analytics/SegmentTest.java index f3dee8a7..768e6331 100644 --- a/analytics/src/test/java/com/segment/analytics/SegmentTest.java +++ b/analytics/src/test/java/com/segment/analytics/SegmentTest.java @@ -9,36 +9,23 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.github.tomakehurst.wiremock.stubbing.ServeEvent; -import com.segment.analytics.internal.AnalyticsClient; import com.segment.analytics.internal.Config; import com.segment.analytics.internal.Config.FileConfig; import com.segment.analytics.internal.Config.HttpConfig; -import com.segment.analytics.internal.FallbackAppender; import com.segment.analytics.messages.TrackMessage; import java.io.IOException; import java.nio.file.Path; -import java.util.Arrays; -import java.util.Collection; +import java.time.Duration; import java.util.HashSet; import java.util.Iterator; import java.util.Map; import java.util.Set; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingDeque; -import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.ConsoleHandler; -import java.util.logging.Level; -import java.util.logging.LogRecord; -import java.util.logging.Logger; -import java.util.logging.SimpleFormatter; -import okhttp3.Dispatcher; -import okhttp3.OkHttpClient; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.RandomStringUtils; import org.awaitility.Awaitility; @@ -53,170 +40,135 @@ public class SegmentTest { + public int requestsPerSecond = 1_000; + public int numClients = 10; + public int messageContentChars = 100; + public int responseDelay = 300; + @Rule - public WireMockRule wireMockRule = new WireMockRule( - wireMockConfig() - .port(8088) - // .dynamicPort() - .gzipDisabled(true), - false); + public WireMockRule wireMockRule = + new WireMockRule(wireMockConfig().port(8088).gzipDisabled(true), false); Analytics analytics; @Before - public void confWireMockAndClient() throws IOException { + public void setup() throws IOException { FileUtils.deleteDirectory(Path.of(Config.DEFAULT_FALLBACK_FILE).toFile()); - stubFor(post(urlEqualTo("/v1/import/")).willReturn(okJson("{\"success\": \"true\"}"))); - - // from SegmentQueue - Integer SEGMENT_FLUSH_QUEUE_SIZE_DEFAULT = 50; - Integer SEGMENT_QUEUE_SIZE_DEFAULT = 250; - Integer DEFAULT_FLUSH_PERIOD_IN_SECONDS = 10; - - // from SegmentQueue getBoundedNetworkExecutor - Integer SEGMENT_EXECUTOR_QUEU_SIZE_DEFAULT = 0; // 5; - Integer EXECUTOR_SIZE = 1; - Integer EXECUTOR_KEEPALIVE_SECONDS = 15; - - ThreadPoolExecutor boundedNetworkExecutor = new ThreadPoolExecutor( - EXECUTOR_SIZE, // corePoolSize - EXECUTOR_SIZE, // maximumPoolSize - EXECUTOR_KEEPALIVE_SECONDS, // keepAliveTime - TimeUnit.SECONDS, - SEGMENT_EXECUTOR_QUEU_SIZE_DEFAULT == 0 - ? new SynchronousQueue<>(true) - : new ArrayBlockingQueue<>(SEGMENT_EXECUTOR_QUEU_SIZE_DEFAULT, true), - // this will cause the HTTP requests to be handled on AnalyticsClient.Looper - // SegmentQueue was discarding oldest tasks // e.getQueue().poll(); e.execute(r); - new CallerRunsPolicy() { - public void rejectedExecution(Runnable r, ThreadPoolExecutor e) { - // System.err.println("==== Pool exhausted ===="); - super.rejectedExecution(r, e); - } - ; - }); - - // segment Platform getDefaultClient - Integer HTTP_TIMEOUT_SECONDS = 15; - OkHttpClient client = new OkHttpClient.Builder() - .connectTimeout(HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) - .readTimeout(HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) - .writeTimeout(HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) - // addedd bounded executor - .dispatcher(new Dispatcher(boundedNetworkExecutor)) - .build(); - - // Then it always add the user-agent interceptor - // FIXME not forcing tls - analytics = Analytics.builder("write-key") .endpoint(wireMockRule.baseUrl()) - .client(client) - .networkExecutor(boundedNetworkExecutor) .httpConfig(HttpConfig.builder() - .flushQueueSize(SEGMENT_FLUSH_QUEUE_SIZE_DEFAULT) - .queueSize(SEGMENT_QUEUE_SIZE_DEFAULT) - .flushIntervalInMillis(DEFAULT_FLUSH_PERIOD_IN_SECONDS * 1_000) + // .queueSize(250) + // .flushQueueSize(50) + // .flushIntervalInMillis(10 * 1_000) + // .executorSize(1) + // .executorQueueSize(0) + // .timeoutSeconds(15) .build()) .fileConfig(FileConfig.builder() - //.size(10_000).flushSize(300) - .build()) + // .size(250) + // .flushSize(50) + .build()) .build(); } - @After - public void tearDown() { - analytics.close(); + + @Test + public void testOk() throws Throwable { + run(Duration.ofSeconds(30), new TimedAction(Duration.ZERO, () -> segmentHttpUp())); } - static void configLogger() { - ConsoleHandler console = new ConsoleHandler(); - console.setLevel(Level.ALL); - console.setFormatter(new SimpleFormatter() { - @Override - public String format(LogRecord record) { - return String.format("[%1$tT.%1$tL] %2$s %n", record.getMillis(), record.getMessage()); - } - }); - - Logger l1 = Logger.getLogger(FallbackAppender.class.getName()); - Logger l2 = Logger.getLogger(AnalyticsClient.class.getName()); - l1.setLevel(Level.ALL); - l1.addHandler(console); - l2.setLevel(Level.ALL); - l2.addHandler(console); + @Test + public void testFailRestoreAtEnd() throws Throwable { + Duration duration = Duration.ofSeconds(30); + run( + duration, + new TimedAction(Duration.ZERO, () -> segmentHttpDown()), + new TimedAction(duration, () -> segmentHttpUp())); } @Test - public void test() throws Throwable { - configLogger(); - - int requestsPerSecond = 1_000; - int numClients = 10; - int messageContentChars = 100; - - int timeToRun = 60_000 * 2; - int timeToRestore = 60_000 * 1; - - int responseDelay = 300; - - stubFor(post(urlEqualTo("/v1/import/")) - .willReturn( - WireMock.aResponse().withStatus(503).withBody("fail").withFixedDelay(responseDelay))); + public void testFailThenRestore() throws Throwable { + run( + Duration.ofMinutes(2), + new TimedAction(Duration.ZERO, () -> segmentHttpDown()), + new TimedAction(Duration.ofMinutes(1), () -> segmentHttpUp())); + } + private void run(Duration durationToRun, TimedAction... actions) throws Throwable { + long timeToRun = durationToRun.toMillis(); long start = System.currentTimeMillis(); - boolean upAgain = false; + + final String content = RandomStringUtils.randomAlphanumeric(messageContentChars); final AtomicInteger id = new AtomicInteger(0); - Collection ids = new ConcurrentLinkedQueue<>(); + ExecutorService exec = new ThreadPoolExecutor( + numClients, + numClients, + 15l, + TimeUnit.SECONDS, + new LinkedBlockingDeque<>(10_000), + new CallerRunsPolicy()); + RateLimiter rate = RateLimiter.create(requestsPerSecond); + int actionIndex = 0; + while (true) { + long elapsed = System.currentTimeMillis() - start; - ExecutorService exec = new ThreadPoolExecutor(numClients, numClients, 15l, TimeUnit.SECONDS, - new LinkedBlockingDeque<>(10_000), new CallerRunsPolicy()); + if (actionIndex < actions.length && actions[actionIndex].at <= elapsed) { + actions[actionIndex].action.run(); + actionIndex++; + } + + if (elapsed > timeToRun) { + break; + } - while (System.currentTimeMillis() - start < timeToRun) { if (rate.tryAcquire()) { exec.submit(() -> { - String msgid = "m" + id.getAndIncrement(); - ids.add(msgid); - analytics.enqueue( - TrackMessage.builder("my-track").messageId(msgid).userId("userId") - .context(Map.of("content", RandomStringUtils.randomAlphanumeric(messageContentChars)))); + String msgid = String.valueOf(id.getAndIncrement()); + analytics.enqueue(TrackMessage.builder("my-track") + .messageId(msgid) + .userId("userId") + .context(Map.of("content", content))); }); } - - if (!upAgain && System.currentTimeMillis() - start > timeToRestore) { - upAgain = true; - stubFor(post(urlEqualTo("/v1/import/")) - .willReturn(okJson("{\"success\": \"true\"}").withFixedDelay(responseDelay))); - System.err.println("UP AGAIN"); - } + Thread.yield(); } - exec.shutdown(); - exec.awaitTermination(10, TimeUnit.MINUTES); + exec.shutdown(); + exec.awaitTermination(10, TimeUnit.MINUTES); + Awaitility.await() + .atMost(10, TimeUnit.MINUTES) + .pollInterval(1, TimeUnit.SECONDS) + .until(() -> checkSentMessages(id.get())); + } - Awaitility.await() - .atMost(10, TimeUnit.MINUTES) - .pollInterval(1, TimeUnit.SECONDS) - .until(() -> sentMessagesEqualsTo(ids.toArray(new String[ids.size()]))); + void segmentHttpUp() { + stubFor(post(urlEqualTo("/v1/import/")) + .willReturn(okJson("{\"success\": \"true\"}").withFixedDelay(responseDelay))); + System.err.println("HTTP server UP"); + } + void segmentHttpDown() { + stubFor(post(urlEqualTo("/v1/import/")) + .willReturn( + WireMock.aResponse().withStatus(503).withBody("fail").withFixedDelay(responseDelay))); + System.err.println("HTTP server DOWN"); } private static final ObjectMapper OM = new ObjectMapper(); - private boolean sentMessagesEqualsTo(String... msgIds) { - Set sentMessages = sentMessages(); - System.err.println("Confirmed msgs %d / %d ".formatted(sentMessages.size(), msgIds.length)); - - return sentMessages.equals(new HashSet<>(Arrays.asList(msgIds))); + private boolean checkSentMessages(int expected) { + int sentMessages = countSendMessages(); + System.err.println("Confirmed msgs %d / %d ".formatted(sentMessages, expected)); + return sentMessages >= expected; } - private Set sentMessages() { - int count = 0; - Set messageIds = new HashSet<>(); + private int countSendMessages() { + int count = 0; + Set messageIds = new HashSet<>(); for (ServeEvent event : wireMockRule.getAllServeEvents()) { if (event.getResponse().getStatus() != 200) { continue; @@ -234,13 +186,29 @@ private Set sentMessages() { } Iterator msgs = batch.elements(); while (msgs.hasNext()) { - count++; - messageIds.add(msgs.next().get("messageId").asText()); + count++; + messageIds.add(msgs.next().get("messageId").asInt()); } } - if (count != messageIds.size()) { - System.err.println(String.format("Duplicates!, count: %d messageIds: %d", count, messageIds.size())); - } - return messageIds; + if (count != messageIds.size()) { + System.err.println(String.format("Duplicates!, count: %d messageIds: %d", count, messageIds.size())); + } + return messageIds.size(); + } + + @After + public void tearDown() { + analytics.close(); + } + + static class TimedAction { + final long at; + final Runnable action; + + public TimedAction(Duration at, Runnable run) { + super(); + this.at = at.toMillis(); + this.action = run; + } } } diff --git a/analytics/src/test/resources/logging.properties b/analytics/src/test/resources/logging.properties new file mode 100644 index 00000000..1c7c7d3a --- /dev/null +++ b/analytics/src/test/resources/logging.properties @@ -0,0 +1,7 @@ +.level=ALL + +handlers=java.util.logging.ConsoleHandler +java.util.logging.ConsoleHandler.level=ALL +java.util.logging.ConsoleHandler.filter= +java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter +java.util.logging.SimpleFormatter.format=%1$tH:%1$tM:%1$tS.%1$tL - %5$s%6$s%n \ No newline at end of file