calling: update TurnCallRouter to reduce returned options

service/src/main/java/org/whispersystems/textsecuregcm/calls/routing/TurnCallRouter.java

@@ -13,6 +13,7 @@ import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.util.Util;
 import javax.annotation.Nonnull;
 import java.io.IOException;
+import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.util.*;
 import java.util.function.Supplier;
@@ -47,7 +48,9 @@ public class TurnCallRouter {
   }
 
   /**
-   * Gets Turn Instance addresses. Returns both the IPv4 and IPv6 addresses. Prioritizes V4 connections.
+   * Gets Turn Instance addresses. Returns both the IPv4 and IPv6 addresses. Prefers to match the IP protocol of the
+   * client address in datacenter selection. Returns 2 instance options of the preferred protocol for every one instance
+   * of the other.
    * @param aci aci of client
    * @param clientAddress IP address to base routing on
    * @param instanceLimit max instances to return options for
@@ -110,29 +113,42 @@ public class TurnCallRouter {
           subdivision
       );
     }
-    List<String> urlsWithIps = getUrlsForInstances(selectInstances(datacenters, instanceLimit));
+
-    return new TurnServerOptions(hostname, urlsWithIps, this.configTurnRouter.randomUrls());
+    List<String> urlsWithIps = getUrlsForInstances(
+        selectInstances(
+            datacenters,
+            instanceLimit,
+            (clientAddress.get() instanceof Inet6Address)
+        ));
+    return new TurnServerOptions(hostname, urlsWithIps, minimalRandomUrls());
   }
 
-  private List<String> selectInstances(List<String> datacenters, int limit) {
+  // Includes only the udp options in the randomUrls
+  private List<String> minimalRandomUrls(){
+    return this.configTurnRouter.randomUrls().stream()
+        .filter(s -> s.startsWith("turn:") && !s.endsWith("transport=tcp"))
+        .toList();
+  }
+
+  private List<String> selectInstances(List<String> datacenters, int limit, boolean preferV6) {
     if(datacenters.isEmpty() || limit == 0) {
       return Collections.emptyList();
     }
+    int numV6 = preferV6 ? (limit - limit / 3) : limit / 3;
+    int numV4  = limit - numV6;
 
     CallDnsRecords dnsRecords = this.callDnsRecords.get();
     List<InetAddress> ipv4Selection = datacenters.stream()
-        .flatMap(dc -> Util.randomNOfStable(dnsRecords.aByRegion().get(dc), 2).stream())
+        .flatMap(dc -> Util.randomNOfStable(dnsRecords.aByRegion().get(dc), limit).stream())
         .toList();
     List<InetAddress> ipv6Selection = datacenters.stream()
-        .flatMap(dc -> Util.randomNOfStable(dnsRecords.aaaaByRegion().get(dc), 2).stream())
+        .flatMap(dc -> Util.randomNOfStable(dnsRecords.aaaaByRegion().get(dc), limit).stream())
         .toList();
-    if (ipv4Selection.size() < ipv6Selection.size()) {
+
-      ipv4Selection = ipv4Selection.stream().limit(limit / 2).toList();
+    // increase numV4 if not enough v6 options. vice-versa is also true
-      ipv6Selection = ipv6Selection.stream().limit(limit - ipv4Selection.size()).toList();
+    numV4 = Math.max(numV4, limit - ipv6Selection.size());
-    } else {
+    ipv4Selection = ipv4Selection.stream().limit(numV4).toList();
-      ipv6Selection = ipv6Selection.stream().limit(limit / 2).toList();
+    ipv6Selection = ipv6Selection.stream().limit(limit - ipv4Selection.size()).toList();
-      ipv4Selection = ipv4Selection.stream().limit(limit - ipv6Selection.size()).toList();
-    }
 
     return Stream.concat(
         ipv4Selection.stream().map(InetAddress::getHostAddress),
@@ -143,7 +159,6 @@ public class TurnCallRouter {
 
   private static List<String> getUrlsForInstances(List<String> instanceIps) {
     return instanceIps.stream().flatMap(ip -> Stream.of(
-            String.format("stun:%s", ip),
             String.format("turn:%s", ip),
             String.format("turn:%s:80?transport=tcp", ip),
             String.format("turns:%s:443?transport=tcp", ip)

service/src/main/java/org/whispersystems/textsecuregcm/controllers/CallRoutingController.java

@@ -33,7 +33,7 @@ import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
 @io.swagger.v3.oas.annotations.tags.Tag(name = "Calling")
 public class CallRoutingController {
 
-  private static final int TURN_INSTANCE_LIMIT = 6;
+  private static final int TURN_INSTANCE_LIMIT = 3;
   private static final Counter INVALID_IP_COUNTER = Metrics.counter(name(CallRoutingController.class, "invalidIP"));
   private static final Logger log = LoggerFactory.getLogger(CallRoutingController.class);
   private final RateLimiters rateLimiters;

service/src/test/java/org/whispersystems/textsecuregcm/calls/routing/TurnCallRouterTest.java

@@ -33,9 +33,12 @@ public class TurnCallRouterTest {
 
   private final static String TEST_HOSTNAME = "subdomain.example.org";
   private final static List<String> TEST_URLS_WITH_HOSTS = List.of(
-      "one.example.com",
+      "stun:one.example.com",
-      "two.example.com",
+      "turn:two.example.com",
-      "three.example.com"
+      "turn:three.example.com?transport=tcp"
+  );
+  private final static List<String> EXPECTED_TEST_URLS_WITH_HOSTS = List.of(
+      "turn:two.example.com"
   );
 
   private CallRoutingTable performanceTable;
@@ -113,7 +116,7 @@ public class TurnCallRouterTest {
     return new TurnServerOptions(
         TEST_HOSTNAME,
         urls,
-        TEST_URLS_WITH_HOSTS
+        EXPECTED_TEST_URLS_WITH_HOSTS
     );
   }
 
@@ -140,7 +143,11 @@ public class TurnCallRouterTest {
         .thenReturn(true);
 
     assertThat(router().getRoutingFor(aci, Optional.of(InetAddress.getByName("0.0.0.1")), 10))
-        .isEqualTo(optionsWithUrls(null));
+        .isEqualTo(new TurnServerOptions(
+            TEST_HOSTNAME,
+            null,
+            TEST_URLS_WITH_HOSTS
+        ));
   }
 
   @Test
@@ -150,32 +157,26 @@ public class TurnCallRouterTest {
 
     assertThat(router().getRoutingFor(aci, Optional.of(InetAddress.getByName("0.0.0.1")), 10))
         .isEqualTo(optionsWithUrls(List.of(
-            "stun:9.9.9.3",
             "turn:9.9.9.3",
             "turn:9.9.9.3:80?transport=tcp",
             "turns:9.9.9.3:443?transport=tcp",
 
-            "stun:9.9.9.1",
             "turn:9.9.9.1",
             "turn:9.9.9.1:80?transport=tcp",
             "turns:9.9.9.1:443?transport=tcp",
 
-            "stun:9.9.9.2",
             "turn:9.9.9.2",
             "turn:9.9.9.2:80?transport=tcp",
             "turns:9.9.9.2:443?transport=tcp",
 
-            "stun:[2222:1111:0:abc2:0:0:0:0]",
             "turn:[2222:1111:0:abc2:0:0:0:0]",
             "turn:[2222:1111:0:abc2:0:0:0:0]:80?transport=tcp",
             "turns:[2222:1111:0:abc2:0:0:0:0]:443?transport=tcp",
 
-            "stun:[2222:1111:0:abc0:0:0:0:0]",
             "turn:[2222:1111:0:abc0:0:0:0:0]",
             "turn:[2222:1111:0:abc0:0:0:0:0]:80?transport=tcp",
             "turns:[2222:1111:0:abc0:0:0:0:0]:443?transport=tcp",
 
-            "stun:[2222:1111:0:abc1:0:0:0:0]",
             "turn:[2222:1111:0:abc1:0:0:0:0]",
             "turn:[2222:1111:0:abc1:0:0:0:0]:80?transport=tcp",
             "turns:[2222:1111:0:abc1:0:0:0:0]:443?transport=tcp"
@@ -191,12 +192,10 @@ public class TurnCallRouterTest {
 
     assertThat(router().getRoutingFor(aci, Optional.of(InetAddress.getByName("0.0.0.1")), 10))
         .isEqualTo(optionsWithUrls(List.of(
-            "stun:1.1.1.1",
             "turn:1.1.1.1",
             "turn:1.1.1.1:80?transport=tcp",
             "turns:1.1.1.1:443?transport=tcp",
 
-            "stun:[2222:1111:0:dead:0:0:0:0]",
             "turn:[2222:1111:0:dead:0:0:0:0]",
             "turn:[2222:1111:0:dead:0:0:0:0]:80?transport=tcp",
             "turns:[2222:1111:0:dead:0:0:0:0]:443?transport=tcp"
@@ -204,41 +203,38 @@ public class TurnCallRouterTest {
   }
 
   @Test
-  public void testLimitReturnsHalfIpv4AndPrioritizesPerformance() throws UnknownHostException {
+  public void testLimitReturnsPreferredProtocolAndPrioritizesPerformance() throws UnknownHostException {
     when(performanceTable.getDatacentersFor(any(), any(), any(), any()))
         .thenReturn(List.of("dc-performance3", "dc-performance2", "dc-performance1"));
 
-    assertThat(router().getRoutingFor(aci, Optional.of(InetAddress.getByName("0.0.0.1")), 6))
+    assertThat(router().getRoutingFor(aci, Optional.of(InetAddress.getByName("0.0.0.1")), 3))
         .isEqualTo(optionsWithUrls(List.of(
-            "stun:9.9.9.4",
             "turn:9.9.9.4",
             "turn:9.9.9.4:80?transport=tcp",
             "turns:9.9.9.4:443?transport=tcp",
 
-            "stun:9.9.9.3",
             "turn:9.9.9.3",
             "turn:9.9.9.3:80?transport=tcp",
             "turns:9.9.9.3:443?transport=tcp",
 
-            "stun:9.9.9.1",
+            "turn:[2222:1111:0:abc3:0:0:0:0]",
-            "turn:9.9.9.1",
+            "turn:[2222:1111:0:abc3:0:0:0:0]:80?transport=tcp",
-            "turn:9.9.9.1:80?transport=tcp",
+            "turns:[2222:1111:0:abc3:0:0:0:0]:443?transport=tcp"
-            "turns:9.9.9.1:443?transport=tcp",
+        )));
+
+    assertThat(router().getRoutingFor(aci, Optional.of(InetAddress.getByName("2222:1111:0:abc2:0:0:0:1")), 3))
+        .isEqualTo(optionsWithUrls(List.of(
+            "turn:9.9.9.4",
+            "turn:9.9.9.4:80?transport=tcp",
+            "turns:9.9.9.4:443?transport=tcp",
 
-            "stun:[2222:1111:0:abc3:0:0:0:0]",
             "turn:[2222:1111:0:abc3:0:0:0:0]",
             "turn:[2222:1111:0:abc3:0:0:0:0]:80?transport=tcp",
             "turns:[2222:1111:0:abc3:0:0:0:0]:443?transport=tcp",
 
-            "stun:[2222:1111:0:abc2:0:0:0:0]",
             "turn:[2222:1111:0:abc2:0:0:0:0]",
             "turn:[2222:1111:0:abc2:0:0:0:0]:80?transport=tcp",
-            "turns:[2222:1111:0:abc2:0:0:0:0]:443?transport=tcp",
+            "turns:[2222:1111:0:abc2:0:0:0:0]:443?transport=tcp"
-
-            "stun:[2222:1111:0:abc0:0:0:0:0]",
-            "turn:[2222:1111:0:abc0:0:0:0:0]",
-            "turn:[2222:1111:0:abc0:0:0:0:0]:80?transport=tcp",
-            "turns:[2222:1111:0:abc0:0:0:0:0]:443?transport=tcp"
         )));
   }