categorize todos

xds/src/main/java/io/grpc/xds/Filter.java

@@ -52,8 +52,8 @@ interface Filter {
 
   default void shutdown() {
     // Implement as needed.
-    // TODO(sergiitk): important to cover and discuss in the design.
-    // TODO(sergiitk): should it be in ServerInterceptorBuilder?
+    // TODO(sergiitk): [DESIGN] important to cover and discuss in the design.
+    // TODO(sergiitk): [QUESTION] should it be in ServerInterceptorBuilder?
   }
 
   /** Represents an opaque data structure holding configuration for a filter. */

xds/src/main/java/io/grpc/xds/RlqsFilter.java

@@ -97,8 +97,7 @@ public ServerInterceptor buildServerInterceptor(
       FilterConfig config,
       @Nullable FilterConfig overrideConfig,
       ScheduledExecutorService scheduler) {
-    // called when we get an xds update - when the LRS or RLS changes.
-    // TODO(sergiitk): this needs to be confirmed.
+    // Called when we get an xds update - when the LRS or RLS changes.
     RlqsFilterConfig rlqsFilterConfig = (RlqsFilterConfig) checkNotNull(config, "config");
 
     // Per-route and per-host configuration overrides.
@@ -119,7 +118,8 @@ public ServerInterceptor buildServerInterceptor(
 
   @Override
   public void shutdown() {
-    // TODO(sergiitk): besides shutting down everything, should there be a per-route destructor?
+    // TODO(sergiitk): [DESIGN] besides shutting down everything, should there
+    //    be per-route interceptor destructors?
     RlqsClientPool oldClientPool = rlqsClientPoolRef.getAndUpdate(unused -> null);
     if (oldClientPool != null) {
       oldClientPool.shutdown();
@@ -152,7 +152,7 @@ public <ReqT, RespT> Listener<ReqT> interceptCall(
         //   - and interface to notify service interceptor on shutdown
         //   - destroy channel when ref count 0
         // potentially many RLQS Clients sharing a channel to grpc RLQS service -
-        //   TODO(sergiitk): look up how cache is looked up
+        //   TODO(sergiitk): [QUESTION] look up how cache is looked up
         // now we create filters every RPC. will be change in RBAC.
         //    we need to avoid recreating filter when config doesn't change
         //    m: trigger close() after we create new instances
@@ -188,7 +188,7 @@ static RlqsFilterConfig parseRlqsFilter(RateLimitQuotaFilterConfig rlqsFilterPro
     builder.domain(rlqsFilterProto.getDomain())
         .rlqsService(GrpcService.fromEnvoyProto(rlqsFilterProto.getRlqsServer()));
 
-    // TODO(sergiitk): bucket_matchers.
+    // TODO(sergiitk): [IMPL] bucket_matchers.
 
     return builder.build();
   }
@@ -197,7 +197,7 @@ static RlqsFilterConfig parseRlqsFilter(RateLimitQuotaFilterConfig rlqsFilterPro
   static RlqsFilterConfig parseRlqsFilterOverride(RateLimitQuotaOverride rlqsFilterProtoOverride)
       throws ResourceInvalidException {
     RlqsFilterConfig.Builder builder = RlqsFilterConfig.builder();
-    // TODO(sergiitk): bucket_matchers.
+    // TODO(sergiitk): [IMPL] bucket_matchers.
 
     return builder.domain(rlqsFilterProtoOverride.getDomain()).build();
   }

xds/src/main/java/io/grpc/xds/RlqsFilterConfig.java

@@ -50,5 +50,5 @@ abstract static class Builder {
     abstract RlqsFilterConfig build();
   }
 
-  // TODO(sergiitk): add rlqs_server, bucket_matchers.
+  // TODO(sergiitk): [IMPL] add rlqs_server, bucket_matchers.
 }

xds/src/main/java/io/grpc/xds/XdsServerWrapper.java

@@ -455,7 +455,7 @@ private void shutdown() {
 
     private void updateSelector() {
       Map<FilterChain, AtomicReference<ServerRoutingConfig>> filterChainRouting = new HashMap<>();
-      // TODO(sergiitk): is this a good place to reset interceptors?
+      // TODO(sergiitk): [QUESTION] is this a good place to reset interceptors?
       // for (FilterChain filterChain : savedRdsRoutingConfigRef.keySet()) {
       //   if (!resourceName.equals(filterChain.httpConnectionManager().rdsName())) {
       //     continue;

xds/src/main/java/io/grpc/xds/internal/datatype/GrpcService.java

@@ -26,12 +26,12 @@
 public abstract class GrpcService {
   public abstract String targetUri();
 
-  // TODO(sergiitk): do we need this?
+  // TODO(sergiitk): [QUESTION] do we need this?
   // abstract String statPrefix();
 
-  // TODO(sergiitk): channelCredentials
-  // TODO(sergiitk): callCredentials
-  // TODO(sergiitk): channelArgs
+  // TODO(sergiitk): [IMPL] channelCredentials
+  // TODO(sergiitk): [IMPL] callCredentials
+  // TODO(sergiitk): [IMPL] channelArgs
 
   /** Optional timeout duration for the gRPC request to the service. */
   @Nullable
@@ -54,10 +54,10 @@ public static GrpcService fromEnvoyProto(
         grpcServiceProto.getGoogleGrpc();
     builder.targetUri(googleGrpcProto.getTargetUri());
 
-    // TODO(sergiitk): channelCredentials
-    // TODO(sergiitk): callCredentials
-    // TODO(sergiitk): channelArgs
-    // TODO(sergiitk): statPrefix - (maybe)
+    // TODO(sergiitk): [IMPL] channelCredentials
+    // TODO(sergiitk): [IMPL] callCredentials
+    // TODO(sergiitk): [IMPL] channelArgs
+    // TODO(sergiitk): [IMPL] statPrefix - (maybe)
 
     return builder.build();
   }

xds/src/main/java/io/grpc/xds/internal/rlqs/RlqsClient.java

@@ -31,6 +31,6 @@ final class RlqsClient {
 
   public void shutdown() {
     logger.log(Level.FINER, "Shutting down RlqsClient to {0}", targetUri);
-    // TODO(sergiitk): impl
+    // TODO(sergiitk): [IMPL] RlqsClient shutdown
   }
 }

xds/src/main/java/io/grpc/xds/internal/rlqs/RlqsClientPool.java

@@ -32,10 +32,9 @@
 public final class RlqsClientPool {
   private static final Logger logger = Logger.getLogger(RlqsClientPool.class.getName());
 
-  // TODO(sergiitk): make a param?
   private static final int DEFAULT_CLEANUP_INTERVAL_SECONDS = 10;
 
-  // TODO(sergiitk): always in sync context?
+  // TODO(sergiitk): [QUESTION] always in sync context?
   private boolean shutdown;
   private final SynchronizationContext syncContext = new SynchronizationContext((thread, error) -> {
     String message = "Uncaught exception in RlqsClientPool SynchronizationContext. Panic!";
@@ -58,8 +57,8 @@ private RlqsClientPool(ScheduledExecutorService scheduler, int cleanupIntervalSe
 
   /** Creates an instance. */
   public static RlqsClientPool newInstance(ScheduledExecutorService scheduler) {
-    // TODO(sergiitk): scheduler - consider using GrpcUtil.TIMER_SERVICE.
-    // TODO(sergiitk): note that the scheduler has a finite lifetime.
+    // TODO(sergiitk): [IMPL] scheduler - consider using GrpcUtil.TIMER_SERVICE.
+    // TODO(sergiitk): [IMPL] note that the scheduler has a finite lifetime.
     return new RlqsClientPool(scheduler, 0);
   }