Skip to content

Commit

Permalink
Merge branch 'main' into feature/client-context-cancel
Browse files Browse the repository at this point in the history
  • Loading branch information
jrhee17 authored Jul 5, 2024
2 parents 3b8958a + 72ebfe1 commit 59aa40a
Show file tree
Hide file tree
Showing 32 changed files with 359 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,24 +147,20 @@ protected final void initialize() {

private void refreshEndpoints(List<Endpoint> endpoints) {
// Allow subclasses to update the endpoints first.
updateNewEndpoints(endpoints).handle((ignored, e) -> {
lock.lock();
try {
pendingFutures.removeIf(ListeningFuture::tryComplete);
} finally {
lock.unlock();
}
return null;
});
updateNewEndpoints(endpoints);
lock.lock();
try {
pendingFutures.removeIf(ListeningFuture::tryComplete);
} finally {
lock.unlock();
}
}

/**
* Invoked when the {@link EndpointGroup} has been updated.
*/
@UnstableApi
protected CompletableFuture<Void> updateNewEndpoints(List<Endpoint> endpoints) {
return UnmodifiableFuture.completedFuture(null);
}
protected void updateNewEndpoints(List<Endpoint> endpoints) {}

private void addPendingFuture(ListeningFuture future) {
lock.lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
Expand All @@ -50,6 +49,7 @@
import com.linecorp.armeria.common.CommonPools;
import com.linecorp.armeria.common.util.ListenableAsyncCloseable;
import com.linecorp.armeria.common.util.Ticker;
import com.linecorp.armeria.internal.common.util.ReentrantShortLock;

import io.netty.util.concurrent.EventExecutor;
import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap;
Expand Down Expand Up @@ -134,6 +134,7 @@ final class RampingUpEndpointWeightSelector extends AbstractEndpointSelector {
@VisibleForTesting
final Map<Long, EndpointsRampingUpEntry> rampingUpWindowsMap = new HashMap<>();
private Object2LongOpenHashMap<Endpoint> endpointCreatedTimestamps = new Object2LongOpenHashMap<>();
private final ReentrantShortLock lock = new ReentrantShortLock(true);

RampingUpEndpointWeightSelector(EndpointGroup endpointGroup, EventExecutor executor) {
super(endpointGroup);
Expand All @@ -145,9 +146,14 @@ final class RampingUpEndpointWeightSelector extends AbstractEndpointSelector {
}

@Override
protected CompletableFuture<Void> updateNewEndpoints(List<Endpoint> endpoints) {
// Use the executor so the order of endpoints change is guaranteed.
return CompletableFuture.runAsync(() -> updateEndpoints(endpoints), executor);
protected void updateNewEndpoints(List<Endpoint> endpoints) {
// Use a lock so the order of endpoints change is guaranteed.
lock.lock();
try {
updateEndpoints(endpoints);
} finally {
lock.unlock();
}
}

private long computeCreateTimestamp(Endpoint endpoint) {
Expand Down Expand Up @@ -245,14 +251,19 @@ private long initialDelayNanos(long windowIndex) {
}

private void updateWeightAndStep(long window) {
final EndpointsRampingUpEntry entry = rampingUpWindowsMap.get(window);
assert entry != null;
final Set<EndpointAndStep> endpointAndSteps = entry.endpointAndSteps();
updateWeightAndStep(endpointAndSteps);
if (endpointAndSteps.isEmpty()) {
rampingUpWindowsMap.remove(window).scheduledFuture.cancel(true);
lock.lock();
try {
final EndpointsRampingUpEntry entry = rampingUpWindowsMap.get(window);
assert entry != null;
final Set<EndpointAndStep> endpointAndSteps = entry.endpointAndSteps();
updateWeightAndStep(endpointAndSteps);
if (endpointAndSteps.isEmpty()) {
rampingUpWindowsMap.remove(window).scheduledFuture.cancel(true);
}
buildEndpointSelector();
} finally {
lock.unlock();
}
buildEndpointSelector();
}

private void updateWeightAndStep(Set<EndpointAndStep> endpointAndSteps) {
Expand All @@ -268,7 +279,12 @@ private void updateWeightAndStep(Set<EndpointAndStep> endpointAndSteps) {
}

private void close() {
rampingUpWindowsMap.values().forEach(e -> e.scheduledFuture.cancel(true));
lock.lock();
try {
rampingUpWindowsMap.values().forEach(e -> e.scheduledFuture.cancel(true));
} finally {
lock.unlock();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import java.util.Comparator;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicInteger;

import com.google.common.collect.ImmutableList;
Expand All @@ -29,7 +28,6 @@
import com.linecorp.armeria.client.ClientRequestContext;
import com.linecorp.armeria.client.Endpoint;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.util.UnmodifiableFuture;

final class WeightedRoundRobinStrategy implements EndpointSelectionStrategy {

Expand Down Expand Up @@ -64,12 +62,11 @@ private static final class WeightedRoundRobinSelector extends AbstractEndpointSe
}

@Override
protected CompletableFuture<Void> updateNewEndpoints(List<Endpoint> endpoints) {
protected void updateNewEndpoints(List<Endpoint> endpoints) {
final EndpointsAndWeights endpointsAndWeights = this.endpointsAndWeights;
if (endpointsAndWeights == null || endpointsAndWeights.endpoints != endpoints) {
this.endpointsAndWeights = new EndpointsAndWeights(endpoints);
}
return UnmodifiableFuture.completedFuture(null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
public class ReentrantShortLock extends ReentrantLock {
private static final long serialVersionUID = 8999619612996643502L;

public ReentrantShortLock() {}

public ReentrantShortLock(boolean fair) {
super(fair);
}

@Override
public void lock() {
super.lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
abstract class AbstractContextPathAnnotatedServiceConfigSetters
<SELF extends AbstractContextPathAnnotatedServiceConfigSetters<SELF, T>,
T extends AbstractContextPathServicesBuilder<?, ?>>
extends AbstractAnnotatedServiceConfigSetters<
AbstractContextPathAnnotatedServiceConfigSetters<SELF, T>> {
extends AbstractAnnotatedServiceConfigSetters<SELF> {

private final T builder;
private final Set<String> contextPaths;
Expand All @@ -42,7 +41,7 @@ abstract class AbstractContextPathAnnotatedServiceConfigSetters
* If path prefix is not set then this service is registered to handle requests matching
* {@code /}
*/
T build(Object service) {
public T build(Object service) {
requireNonNull(service, "service");
service(service);
contextPaths(contextPaths);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
abstract class AbstractContextPathServiceBindingBuilder
<SELF extends AbstractContextPathServiceBindingBuilder<SELF, T>,
T extends AbstractContextPathServicesBuilder<?, ?>>
extends AbstractServiceBindingBuilder<AbstractContextPathServiceBindingBuilder<SELF, T>> {
extends AbstractServiceBindingBuilder<SELF> {

private final T contextPathServicesBuilder;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
* @see VirtualHostServiceBindingBuilder
*/
abstract class AbstractServiceBindingBuilder<SELF extends AbstractServiceBindingBuilder<SELF>>
extends AbstractBindingBuilder<SELF>
implements ServiceConfigSetters<AbstractServiceBindingBuilder<SELF>> {
extends AbstractBindingBuilder<SELF> implements ServiceConfigSetters<SELF> {

private final DefaultServiceConfigSetters defaultServiceConfigSetters = new DefaultServiceConfigSetters();

Expand Down Expand Up @@ -261,14 +260,4 @@ final void build0(HttpService service) {
}
}
}

final void build0(HttpService service, Route mappedRoute) {
final List<Route> routes = buildRouteList(ImmutableSet.of());
assert routes.size() == 1; // Only one route is set via addRoute().
final HttpService decoratedService = defaultServiceConfigSetters.decorator().apply(service);
final ServiceConfigBuilder serviceConfigBuilder =
defaultServiceConfigSetters.toServiceConfigBuilder(routes.get(0), "/", decoratedService);
serviceConfigBuilder.addMappedRoute(mappedRoute);
serviceConfigBuilder(serviceConfigBuilder);
}
}
13 changes: 10 additions & 3 deletions core/src/main/resources/com/linecorp/armeria/public_suffixes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
*.compute.amazonaws.com
*.compute.amazonaws.com.cn
*.compute.estate
*.cprapid.com
*.cryptonomic.net
*.customer-oci.com
*.d.crm.dev
Expand Down Expand Up @@ -1376,6 +1375,7 @@ cf
cf-ipfs.com
cfa
cfd
cfolks.pl
cg
ch
ch.eu.org
Expand Down Expand Up @@ -1822,11 +1822,13 @@ courses
coz.br
cpa
cpa.pro
cprapid.com
cpserver.com
cq.cn
cr
cr.it
cr.ua
craft.me
crafting.xyz
cranky.jp
crap.jp
Expand Down Expand Up @@ -2102,6 +2104,7 @@ duckdns.org
dunlop
dupont
durban
durumis.com
dvag
dvr
dvrcam.info
Expand Down Expand Up @@ -2944,7 +2947,6 @@ fylkesbibl.no
fyresdal.no
g.bg
g.se
g.vbrplsbx.io
g12.br
ga
ga.us
Expand Down Expand Up @@ -3437,6 +3439,12 @@ hasuda.saitama.jp
hasura-app.io
hasura.app
hasvik.no
hateblo.jp
hatenablog.com
hatenablog.jp
hatenadiary.com
hatenadiary.jp
hatenadiary.org
hatinh.vn
hatogaya.saitama.jp
hatoyama.saitama.jp
Expand Down Expand Up @@ -3874,7 +3882,6 @@ ink
ino.kochi.jp
instance.datadetect.com
instances.spawn.cc
instantcloud.cn
institute
insurance
insurance.aero
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright 2024 LINE Corporation
*
* LINE Corporation 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:
*
* https://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.linecorp.armeria.server.contextpath.test;

import org.junit.jupiter.api.Test;

import com.linecorp.armeria.server.ContextPathAnnotatedServiceConfigSetters;
import com.linecorp.armeria.server.ContextPathServiceBindingBuilder;
import com.linecorp.armeria.server.ContextPathServicesBuilder;
import com.linecorp.armeria.server.Server;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.ServiceBindingBuilder;
import com.linecorp.armeria.server.VirtualHostBuilder;
import com.linecorp.armeria.server.VirtualHostContextPathAnnotatedServiceConfigSetters;
import com.linecorp.armeria.server.VirtualHostContextPathServiceBindingBuilder;
import com.linecorp.armeria.server.VirtualHostContextPathServicesBuilder;

class ServiceBuilderSelfTypeTest {

// A non-existent package is used to check if the API is exposed publicly.

@Test
void contextPathAnnotatedServiceConfigSetters() {
final ContextPathAnnotatedServiceConfigSetters setters =
Server.builder()
.contextPath("/foo")
.annotatedService()
.addHeader("X-foo", "bar");
final ContextPathServicesBuilder contextPathServicesBuilder = setters.build(new Object());
final ServerBuilder serverBuilder = contextPathServicesBuilder.and();
}

@Test
void virtualHostContextPathAnnotatedServiceConfigSetters() {
final VirtualHostContextPathAnnotatedServiceConfigSetters setters =
Server.builder()
.virtualHost("foo.com")
.contextPath("/foo")
.annotatedService()
.addHeader("X-foo", "bar");
final VirtualHostContextPathServicesBuilder contextPathServicesBuilder = setters.build(new Object());
final VirtualHostBuilder serverBuilder = contextPathServicesBuilder.and();
}

@Test
void serviceBindingBuilder() {
final ServiceBindingBuilder serviceBindingBuilder =
Server.builder()
.route()
.path("/");
final ServiceBindingBuilder serviceBindingBuilder1 = serviceBindingBuilder.decorator(
(delegate, ctx, req) -> null);
serviceBindingBuilder1.build((ctx, req) -> null);
}

@Test
void contextPathServiceBindingBuilder() {
final ContextPathServiceBindingBuilder builder =
Server.builder()
.contextPath("/foo")
.route()
.path("/")
.addHeader("X-foo", "bar");
builder.build((ctx, req) -> null);
}

@Test
void virtualHostContextPathServiceBindingBuilder() {
final VirtualHostContextPathServiceBindingBuilder builder =
Server.builder()
.virtualHost("foo.com")
.contextPath("/foo")
.route()
.path("/")
.addHeader("X-foo", "bar");
builder.build((ctx, req) -> null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class GrpcExceptionHandler implements GrpcExceptionHandlerFunction {

@Nullable
@Override
public Status apply(RequestContext ctx, @Nullable Status status, Throwable cause, Metadata metadata) {
public Status apply(RequestContext ctx, Status status, Throwable cause, Metadata metadata) {
if (cause instanceof IllegalArgumentException) {
return Status.INVALID_ARGUMENT.withCause(cause);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ internal class CoroutineServerInterceptorTest {
val exceptionHandler =
GrpcExceptionHandlerFunction {
_: RequestContext,
_: Status?,
_: Status,
throwable: Throwable,
_: Metadata,
->
Expand Down
Loading

0 comments on commit 59aa40a

Please sign in to comment.