Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Fix BatchCounter concurrency issue #551

Merged
merged 1 commit into from
Jun 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ public void setPath(String path) {
}

private static class Monitoring {
private Integer batchSize;
private Long batchSize;

public Integer getBatchSize() {
public Long getBatchSize() {
return batchSize;
}

public void setBatchSize(Integer batchSize) {
public void setBatchSize(Long batchSize) {
this.batchSize = batchSize;
}
}
Expand All @@ -135,11 +135,11 @@ public void setMonitoring(Monitoring monitoring) {
this.monitoring = monitoring;
}

public Integer getMonitoringBatchSize() {
public Long getMonitoringBatchSize() {
return this.monitoring.getBatchSize();
}

public void setMonitoringBatchSize(Integer batchSize) {
public void setMonitoringBatchSize(Long batchSize) {
this.monitoring.setBatchSize(batchSize);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.MeterRegistry;
import java.util.concurrent.atomic.AtomicLong;

/**
* Batch counter for counting requests for monitoring. Counts up in batches, given batch size. This way, single requests
Expand All @@ -33,11 +34,11 @@ public class BatchCounter {
private static final String SUBMISSION_CONTROLLER_REQUESTS_COUNTER_DESCRIPTION
= "Counts requests to the Submission Controller.";

private final int batchSize;
private final long batchSize;
private final Counter counter;
private Double batch = 0.;
private final AtomicLong count = new AtomicLong(0L);

BatchCounter(MeterRegistry meterRegistry, int batchSize, String type) {
BatchCounter(MeterRegistry meterRegistry, long batchSize, String type) {
this.batchSize = batchSize;
counter = Counter.builder(SUBMISSION_CONTROLLER_REQUESTS_COUNTER_NAME)
.tag("type", type)
Expand All @@ -50,11 +51,8 @@ public class BatchCounter {
* counter is incremented.
*/
public void increment() {
if (batch < batchSize) {
batch++;
} else {
counter.increment(batch);
batch = 1.;
if (0 == count.incrementAndGet() % batchSize) {
counter.increment(batchSize);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class SubmissionControllerMonitor {

private final MeterRegistry meterRegistry;

private final Integer batchSize;
private final long batchSize;
private BatchCounter requests;
private BatchCounter realRequests;
private BatchCounter fakeRequests;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*-
* ---license-start
* Corona-Warn-App
* ---
* Copyright (C) 2020 SAP SE and all other contributors
* ---
* Licensed 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.
* ---license-end
*/

package app.coronawarn.server.services.submission.monitoring;

import static org.mockito.Mockito.mock;
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 io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.MeterRegistryMock;
import java.util.stream.LongStream;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

class BatchCounterTest {

private static final String COUNTER_TYPE = "FooCounter";
private MeterRegistry meterRegistry;
private Counter meterCounter;

@BeforeEach
void setUpCounter() {
meterCounter = mock(Counter.class);
meterRegistry = spy(new MeterRegistryMock(meterCounter));
}

@ParameterizedTest
@ValueSource(longs = {1L, 2L, 4L})
void incrementSubmittedOnceIfBatchSizeReached(long batchSize) {
BatchCounter batchCounter = new BatchCounter(meterRegistry, batchSize, COUNTER_TYPE);
LongStream.range(0, batchSize).forEach(__ -> batchCounter.increment());
verify(meterCounter, times(1)).increment(batchSize);
}

@ParameterizedTest
@ValueSource(longs = {2L, 4L, 7L})
void doesNotIncrementIfLesserThanBatchSize(long batchSize) {
BatchCounter batchCounter = new BatchCounter(meterRegistry, batchSize, COUNTER_TYPE);
LongStream.range(0, batchSize - 1).forEach(__ -> batchCounter.increment());
verify(meterCounter, never()).increment(batchSize);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*-
* ---license-start
* Corona-Warn-App
* ---
* Copyright (C) 2020 SAP SE and all other contributors
* ---
* Licensed 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.
* ---license-end
*/

package io.micrometer.core.instrument;

import io.micrometer.core.instrument.simple.SimpleMeterRegistry;

/**
* Used to get access to the {@link Counter} instance in BatchCounterTest.
*/
public class MeterRegistryMock extends SimpleMeterRegistry {

final Counter counter;

public MeterRegistryMock(Counter counter) {
this.counter = counter;
}

@Override
Counter counter(Meter.Id id) {
return counter;
}
}