Make the pool key independent of attribute order

Summary:
A squangle `PoolKey` decides whether a connection request can reuse an idle pooled connection. This diff makes two keys with the same connection attributes equal whatever order the attribute map iterates in, so such a request reuses the idle connection instead of opening a new one. It does not depend on any other diff.

**Problem**
A request whose key does not match opens a new connection while an idle one with the same attributes sits in the pool. `PoolKey` hashed the attributes with `folly::hash::hash_range`, which depends on iteration order, and two keys are equal only if those hashes are equal. The attributes have been a `folly::F14NodeMap` since D47197821, and the iteration order of that map is not part of its value:
- In a build without `NDEBUG`, F14 puts each insert in a random slot. Two maps filled with the same four attributes in the same order then give different keys about seven times in ten.
- In any build, two maps filled with the same attributes in a different order give different keys.

**Who sees it**
- `DBQueryTest::testConnectionPooling` in www runs one pooled query twice and expects the second to reuse the connection of the first.
- It fails on diffs that change HHVM sources under `fbcode/hphp`. Their CI builds HHVM in `@//mode/dbgo`, which has no `NDEBUG`, and runs the `flib/core` www tests against that build in the job `hhvm-flib-test-dbgo`.
- That job reported 121 failures of the test on 65 diffs in the two weeks to 2026-10-03. In the 72 hours to 21:36 PDT on 2026-10-03 the job ran the test 59 times without this change: 28 failed, 15 passed only after one or two reruns, and 16 passed with no rerun. D71821697, created in March 2025, called the test "very flaky on changes to HHVM".

**What changes when this lands**
- A binary built without `NDEBUG` that connects through a pool with two or more attributes reuses pooled connections, as a release build already does. That includes debug and dev-mode tests elsewhere in fbcode that use the pool this way, such as the one in Test Plan step 5.
- A release build is unchanged for callers that set the same attributes in the same order. `DBQueryTest` is one: in the same 72 hours it passed with no rerun in all 109 runs of the optimized `hhvm-flib-test` job and all 52 runs of `continuous-hhvm-flib-test`.
- On any build, release included, two callers that pass the same attributes in a different order did not share pooled connections before this change, and do after it. HHVM sets them in the order of the array given to `setConnectionAttributes`.

**Change**
`options_hash_` is now `folly::hash::commutative_hash_combine_range` over the attributes, which does not depend on order. `QueryOptions::hashValue()` in the same library already hashes its attribute map this way. The diff adds `PoolKeyTest`, with five cases, and its BUCK target.

**Unchanged**
- Two keys are still compared by their hashes and their `ConnectionKey`. The attribute maps themselves are not compared.
- The hash values change for keys that have attributes. A key with no attributes keeps an options hash of 0. The hashes are used only as keys of in-memory maps, through `PoolKeyHash` and `PoolKeyPartialHash`.
- No knob gates this. Rollback is a revert of this diff.

Reviewed By: jkedgar

Differential Revision: D123240364

fbshipit-source-id: 02e88be12c1a0a9bbee38b235c2b52a816c47678

squangle/mysql_client/PoolKey.h

@@ -8,6 +8,8 @@
 
 #pragma once
 
+#include <folly/hash/Hash.h>
+
 #include "squangle/base/ConnectionKey.h"
 #include "squangle/mysql_client/ConnectionOptions.h"
 
@@ -21,7 +23,9 @@ class PoolKey {
       std::shared_ptr<const ConnectionKey> conn_key,
       ConnectionOptions conn_opts)
       : connKey_(std::move(conn_key)), connOptions_(std::move(conn_opts)) {
-    options_hash_ = folly::hash::hash_range(
+    // The attributes are an unordered map, so equal sets of attributes have to
+    // hash equally in whatever order they iterate.
+    options_hash_ = folly::hash::commutative_hash_combine_range(
         connOptions_.getAttributes().begin(),
         connOptions_.getAttributes().end());
     partial_hash_ =

squangle/mysql_client/test/PoolKeyTest.cpp

@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include <gtest/gtest.h>
+
+#include "squangle/mysql_client/PoolKey.h"
+#include "squangle/mysql_client/PoolStorage.h"
+#include "squangle/mysql_client/SyncConnectionPool.h"
+#include "squangle/mysql_client/SyncMysqlClient.h"
+#include "squangle/mysql_client/TwoLevelCache.h"
+#include "squangle/mysql_client/test/MockConnection.h"
+
+namespace facebook::common::mysql_client::test {
+
+namespace {
+
+constexpr std::pair<std::string_view, std::string_view> kAttributes[] = {
+    {"role", "web"},
+    {"caller", "DBQueryTest"},
+    {"job", "none"},
+    {"script", "test.php"},
+};
+
+ConnectionOptions optionsWithAttributes(bool reversed = false) {
+  ConnectionOptions opts;
+  const auto count = std::size(kAttributes);
+  for (size_t i = 0; i < count; ++i) {
+    const auto& [name, value] = kAttributes[reversed ? count - 1 - i : i];
+    opts.setAttribute(name, std::string(value));
+  }
+  return opts;
+}
+
+PoolKey keyFor(ConnectionOptions opts) {
+  return PoolKey(MockMysqlClient::createTestConnectionKey(), std::move(opts));
+}
+
+} // namespace
+
+TEST(PoolKeyTest, AttributeOrderDoesNotChangeTheKey) {
+  const auto forward = keyFor(optionsWithAttributes());
+  const auto backward = keyFor(optionsWithAttributes(/* reversed */ true));
+
+  EXPECT_EQ(forward.getOptionsHash(), backward.getOptionsHash());
+  EXPECT_EQ(forward.getHash(), backward.getHash());
+  EXPECT_EQ(forward.getPartialHash(), backward.getPartialHash());
+  EXPECT_TRUE(forward == backward);
+  EXPECT_TRUE(forward.partialCompare(backward));
+}
+
+// A debug build of F14 puts each insert in a random slot, so there two maps
+// filled in the same order can still iterate in different orders.
+TEST(PoolKeyTest, SameAttributesAlwaysGiveTheSameKey) {
+  const auto first = keyFor(optionsWithAttributes());
+  int different = 0;
+  for (int i = 0; i < 1000; ++i) {
+    if (first != keyFor(optionsWithAttributes())) {
+      ++different;
+    }
+  }
+  EXPECT_EQ(different, 0) << "of 1000 keys built from the same attributes";
+}
+
+TEST(PoolKeyTest, DifferentAttributesGiveDifferentKeys) {
+  const auto base = keyFor(optionsWithAttributes());
+
+  EXPECT_TRUE(base != keyFor(ConnectionOptions()));
+  EXPECT_TRUE(
+      base != keyFor(optionsWithAttributes().setAttribute("job", "other")));
+  EXPECT_TRUE(
+      base != keyFor(optionsWithAttributes().setAttribute("extra", "1")));
+
+  // The same names and the same values, paired differently.
+  ConnectionOptions one;
+  one.setAttribute("a", "1").setAttribute("b", "2");
+  ConnectionOptions other;
+  other.setAttribute("a", "2").setAttribute("b", "1");
+  EXPECT_TRUE(keyFor(one) != keyFor(other));
+}
+
+TEST(PoolKeyTest, StorageFindsAWaitingOperationByAnEqualKey) {
+  auto pool = SyncConnectionPool::makePool(std::make_shared<SyncMysqlClient>());
+  const auto connKey = MockMysqlClient::createTestConnectionKey();
+  auto op = std::dynamic_pointer_cast<ConnectPoolOperation<SyncMysqlClient>>(
+      pool->beginConnection(connKey));
+  ASSERT_NE(op, nullptr);
+
+  PoolStorageData<SyncMysqlClient> storage(
+      /* conn_limit */ 10, /* max_idle_time */ std::chrono::seconds(60));
+  storage.queueOperation(PoolKey(connKey, optionsWithAttributes()), op);
+
+  const PoolKey equalKey(connKey, optionsWithAttributes(/* reversed */ true));
+  EXPECT_EQ(storage.popOperation(equalKey), op);
+}
+
+// Idle connections wait in a TwoLevelCache, which also groups keys by
+// PoolKeyPartialHash.
+TEST(PoolKeyTest, CacheFindsAnIdleEntryByAnEqualKey) {
+  struct Value {};
+  TwoLevelCache<
+      SyncMysqlClient,
+      PoolKey,
+      std::unique_ptr<Value>,
+      PoolKeyHash,
+      PoolKeyPartialHash>
+      cache;
+  const auto connKey = MockMysqlClient::createTestConnectionKey();
+  cache.push(
+      PoolKey(connKey, optionsWithAttributes()),
+      std::make_unique<Value>(),
+      /* max */ 10);
+
+  const PoolKey equalKey(connKey, optionsWithAttributes(/* reversed */ true));
+  EXPECT_EQ(cache.level2Size(equalKey), 1);
+  EXPECT_NE(cache.popLevel1(equalKey), nullptr);
+}
+
+} // namespace facebook::common::mysql_client::test