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

Commit

Permalink
Update comments based on the review
Browse files Browse the repository at this point in the history
  • Loading branch information
mutianf committed Feb 25, 2021
1 parent ace3659 commit 53d8d7e
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 34 deletions.
Expand Up @@ -40,38 +40,38 @@
@InternalApi("For google-cloud-java client use only")
public abstract class DynamicFlowControlSettings {

/** Number of outstanding elements that {@link FlowController} will allow when it's initiated. */
/** Number of outstanding elements that {@link FlowController} allows when it's initiated. */
@Nullable
public abstract Long getInitialOutstandingElementCount();

/** Number of outstanding bytes that {@link FlowController} will allow when it's initiated. */
/** Number of outstanding bytes that {@link FlowController} allows when it's initiated. */
@Nullable
public abstract Long getInitialOutstandingRequestBytes();

/**
* Absolute maximum number of outstanding elements {@link FlowController} can allow before
* enforcing flow control.
* Maximum number of outstanding elements {@link FlowController} allows before enforcing flow
* control.
*/
@Nullable
public abstract Long getMaxOutstandingElementCount();

/**
* Absolute maximum number of outstanding bytes {@link FlowController} can allow before enforcing
* flow control.
* Maximum number of outstanding bytes {@link FlowController} allows before enforcing flow
* control.
*/
@Nullable
public abstract Long getMaxOutstandingRequestBytes();

/**
* Absolute minimum number of outstanding elements {@link FlowController} will allow before
* enforcing flow control.
* Minimum number of outstanding elements {@link FlowController} allows before enforcing flow
* control.
*/
@Nullable
public abstract Long getMinOutstandingElementCount();

/**
* Absolute minimum number of outstanding bytes {@link FlowController} will allow before enforcing
* flow control.
* Minimum number of outstanding bytes {@link FlowController} allows before enforcing flow
* control.
*/
@Nullable
public abstract Long getMinOutstandingRequestBytes();
Expand Down
Expand Up @@ -171,7 +171,7 @@ public FlowController(FlowControlSettings settings) {
"Unknown LimitBehaviour: " + settings.getLimitExceededBehavior());
}
// When the FlowController is initialized with FlowControlSettings, flow control limits can't be
// adjusted. min, current, max element count and request bytes will be initialized with the max
// adjusted. min, current, max element count and request bytes are initialized with the max
// values in FlowControlSettings.
this.maxOutstandingElementCount = settings.getMaxOutstandingElementCount();
this.minOutstandingElementCount = settings.getMaxOutstandingElementCount();
Expand Down Expand Up @@ -251,7 +251,7 @@ public void reserve(long elements, long bytes) throws FlowControlException {
}
}

// Will always allow to send a request even if it is larger than the flow control limit,
// Always allows to send a request even if it is larger than the flow control limit,
// if it doesn't then it will deadlock the thread.
if (outstandingByteCount != null) {
long permitsToDraw, permitsOwed;
Expand Down Expand Up @@ -315,8 +315,8 @@ public synchronized void decreaseThresholds(long elementSteps, long byteSteps) {

/**
* Set flow control limits to elements and bytes. If elements or bytes are greater than the
* absolute maximum, limits will be set to the maximum. If elements or bytes are smaller than the
* absolute minimum, limits will be set to the minimum.
* absolute maximum, limits are set to the maximum. If elements or bytes are smaller than the
* absolute minimum, limits are set to the minimum.
*/
@InternalApi("For google-cloud-java client use only")
public synchronized void setThresholds(long elements, long bytes) {
Expand Down
Expand Up @@ -54,7 +54,7 @@ public void release(long permits) {
public boolean acquire(long permits) {
checkNotNegative(permits);

for (; ; ) {
while (true) {
long old = currentPermits.get();
if (old < permits) {
return false;
Expand All @@ -67,7 +67,7 @@ public boolean acquire(long permits) {

public void reducePermits(long reduction) {
checkNotNegative(reduction);
for (; ; ) {
while (true) {
long old = currentPermits.get();
if (currentPermits.compareAndSet(old, old - reduction)) {
return;
Expand Down
Expand Up @@ -30,6 +30,8 @@
package com.google.api.gax.batching;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;

import com.google.api.gax.batching.FlowController.LimitExceededBehavior;
Expand All @@ -44,13 +46,13 @@ public class DynamicFlowControlSettingsTest {
public void testEmptyBuilder() {
DynamicFlowControlSettings.Builder builder = DynamicFlowControlSettings.newBuilder();
DynamicFlowControlSettings settings = builder.build();
assertThat(settings.getInitialOutstandingElementCount()).isNull();
assertThat(settings.getInitialOutstandingRequestBytes()).isNull();
assertThat(settings.getMaxOutstandingElementCount()).isNull();
assertThat(settings.getMaxOutstandingRequestBytes()).isNull();
assertThat(settings.getMinOutstandingElementCount()).isNull();
assertThat(settings.getMinOutstandingRequestBytes()).isNull();
assertThat(settings.getLimitExceededBehavior()).isEqualTo(LimitExceededBehavior.Block);
assertNull(settings.getInitialOutstandingElementCount());
assertNull(settings.getInitialOutstandingRequestBytes());
assertNull(settings.getMaxOutstandingElementCount());
assertNull(settings.getMaxOutstandingRequestBytes());
assertNull(settings.getMinOutstandingElementCount());
assertNull(settings.getMinOutstandingRequestBytes());
assertEquals(LimitExceededBehavior.Block, settings.getLimitExceededBehavior());
}

@Test
Expand Down Expand Up @@ -140,7 +142,7 @@ private void testInvalidElementCount(long initial, long min, long max) {
.setMaxOutstandingElementCount(max);
try {
builder.build();
fail("Must have thrown an illegal argument error");
fail("Did not throw an illegal argument error");
} catch (IllegalArgumentException e) {
// Expected, ignore
}
Expand All @@ -154,7 +156,7 @@ private void testInvalidNumberOfBytes(long initial, long min, long max) {
.setMaxOutstandingRequestBytes(max);
try {
builder.build();
fail("Must have thrown an illegal argument error");
fail("Did not throw an illegal argument error");
} catch (IllegalArgumentException e) {
// Expected, ignore
}
Expand Down
19 changes: 10 additions & 9 deletions gax/src/test/java/com/google/api/gax/batching/Semaphore64Test.java
Expand Up @@ -29,7 +29,8 @@
*/
package com.google.api.gax.batching;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -48,10 +49,10 @@ public void testNegative() {
@Test
public void testReturning() {
Semaphore64 semaphore = new NonBlockingSemaphore(1);
assertThat(semaphore.acquire(1)).isTrue();
assertThat(semaphore.acquire(1)).isFalse();
assertTrue(semaphore.acquire(1));
assertFalse(semaphore.acquire(1));
semaphore.release(1);
assertThat(semaphore.acquire(1)).isTrue();
assertTrue(semaphore.acquire(1));
}

@Test
Expand All @@ -77,24 +78,24 @@ public void run() {
Thread.sleep(500);

for (Thread t : acquirers) {
assertThat(t.isAlive()).isTrue();
assertTrue(t.isAlive());
}

semaphore.release(3);
semaphore.release(3);

for (Thread t : acquirers) {
t.join(500);
assertThat(t.isAlive()).isFalse();
assertFalse(t.isAlive());
}
}

@Test
public void testReducePermitsNonBlocking() {
final Semaphore64 semaphore = new NonBlockingSemaphore(5);
semaphore.reducePermits(3);
assertThat(semaphore.acquire(3)).isFalse();
assertThat(semaphore.acquire(2)).isTrue();
assertFalse(semaphore.acquire(3));
assertTrue(semaphore.acquire(2));
}

@Test(timeout = 500)
Expand All @@ -120,7 +121,7 @@ public void run() {

Thread.sleep(100);
for (Thread t : acquires) {
assertThat(t.isAlive()).isTrue();
assertTrue(t.isAlive());
}

semaphore.release(6);
Expand Down

0 comments on commit 53d8d7e

Please sign in to comment.