Merge bitcoin/bitcoin#35173: util: shorten thread names to avoid Linux truncation

d3e40af259 index: shorten indexer thread names (Lőrinc)
d69c46292d util: zero-pad thread number suffixes (Lőrinc)
41e531c4ab util: shorten `ThreadPool` worker names (Lőrinc)

Pull request description:

  **Problem:** Linux limits thread names set through [`PR_SET_NAME`](https://man7.org/linux/man-pages/man2/PR_SET_NAME.2const.html) to 15 visible bytes:

  > The name can be up to 16 bytes long, including the terminating null byte.

  Bitcoin Core prefixes system thread names with `b-`, leaving only 13 bytes for the thread-specific part.
  This truncates longer indexer names in system tools, for example `b-coinstatsindex` and `b-txospenderindex`.
  It also makes verbose worker suffixes like `b-http_pool_N` spend much of the available space; the current HTTP worker names fit, but the generic suffix leaves less room for longer pool names.

  The same limit is documented in the existing thread-name helper:
  8b49e2dd4e/src/util/threadnames.cpp (L25)

  This was noticed during review of https://github.com/bitcoin/bitcoin/pull/31132#discussion_r3146688138

  **Fix:** Shorten the OS-visible thread names while keeping public index identifiers unchanged.
  `ThreadPool` workers now use a zero-padded dotted numeric suffix, so HTTP workers are named like `b-http.xx`.
  `CCheckQueue` worker suffixes are zero-padded as well, so script-check workers are named like `b-scriptch.xx`.

  Indexer sync threads now pass display and thread names separately at each `BaseIndex` call site.

  The current indexer thread names use compact `idx`-suffixed names that fit within the Linux limit after the `b-` prefix:

  ```text
  txindex                  -> txidx
  basic block filter index -> blkfltbscidx
  coinstatsindex           -> coinstatsidx
  txospenderindex          -> txospenderidx
  ```

  Indexer display names, `getindexinfo` keys, command-line options, and on-disk index paths are unchanged.

  **Testing:** See https://godbolt.org/z/oWonrTKcj for a simple reproducer.

  Alternatively, start `bitcoind` on Linux with the affected indexes enabled and read the kernel-visible thread names.

  Before this change, the relevant names were truncated or used longer forms:

  ```text
  b-txindex
  b-basic block f
  b-coinstatsinde
  b-http_pool_15
  b-txospenderind
  ```

  After this change, the same check shows compact, untruncated names:

  ```text
  b-txidx
  b-blkfltbscidx
  b-coinstatsidx
  b-http.15
  b-txospenderidx
  ```

ACKs for top commit:
  maflcko:
    re-ACK d3e40af259 👳
  sedited:
    ACK d3e40af259
  winterrdog:
    re-ACK d3e40af259
  hodlinator:
    re-ACK d3e40af259

Tree-SHA512: 0b4c087661eb81e767fb2c2a1ce2dd54e6593888a7d30402e76c845a84dff5550e3ad72fee39b136f7f5214f051647c0c1f284e3265f8a614d1028f7b49d76da

doc/developer-notes.md

@@ -699,16 +699,16 @@ and its `cs_KeyStore` lock for example).
   : Performs various loading tasks that are part of init but shouldn't block the node from being started: external block import,
    reindex, reindex-chainstate, main chain activation, spawn indexes background sync threads and mempool load.
 
-- [CCheckQueue::Loop (`b-scriptch.x`)](https://doxygen.bitcoincore.org/class_c_check_queue.html#checkqueue)
+- [CCheckQueue::Loop (`b-scriptch.xx`)](https://doxygen.bitcoincore.org/class_c_check_queue.html#checkqueue)
   : Parallel script validation threads for transactions in blocks.
 
 - [ThreadHTTP (`b-http`)](https://doxygen.bitcoincore.org/httpserver_8cpp.html#http)
   : Libevent thread to listen for RPC and REST connections.
 
-- [HTTP worker threads (`b-http_pool_x`)](https://doxygen.bitcoincore.org/httpserver_8cpp.html#http_pool)
+- [HTTP worker threads (`b-http.xx`)](https://doxygen.bitcoincore.org/httpserver_8cpp.html#http_pool)
   : Threads to service RPC and REST requests.
 
-- [Indexer threads (`b-txindex`, etc)](https://doxygen.bitcoincore.org/class_base_index.html#index_sync)
+- [Indexer threads (`b-txidx`, `b-blkfltbscidx`, `b-coinstatsidx`, `b-txospenderidx`)](https://doxygen.bitcoincore.org/class_base_index.html#index_sync)
   : One thread per indexer.
 
 - [SchedulerThread (`b-scheduler`)](https://doxygen.bitcoincore.org/class_c_scheduler.html#scheduler)

src/checkqueue.h

@@ -149,7 +149,7 @@ public:
         m_worker_threads.reserve(worker_threads_num);
         for (int n = 0; n < worker_threads_num; ++n) {
             m_worker_threads.emplace_back([this, n]() {
-                util::ThreadRename(strprintf("scriptch.%i", n));
+                util::ThreadRename(strprintf("scriptch.%02i", n));
                 Loop(false /* worker thread */);
             });
         }

src/index/base.cpp

@@ -92,8 +92,8 @@ void BaseIndex::DB::WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator
     batch.Write(DB_BEST_BLOCK, locator);
 }
 
-BaseIndex::BaseIndex(std::unique_ptr<interfaces::Chain> chain, std::string name)
-    : m_chain{std::move(chain)}, m_name{std::move(name)} {}
+BaseIndex::BaseIndex(std::unique_ptr<interfaces::Chain> chain, std::string name, std::string thread_name)
+    : m_chain{std::move(chain)}, m_name{std::move(name)}, m_thread_name{std::move(thread_name)} {}
 
 BaseIndex::~BaseIndex()
 {
@@ -460,7 +460,7 @@ bool BaseIndex::StartBackgroundSync()
 {
     if (!m_init) throw std::logic_error("Error: Cannot start a non-initialized index");
 
-    m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { Sync(); });
+    m_thread_sync = std::thread(&util::TraceThread, m_thread_name, [this] { Sync(); });
     return true;
 }
 

src/index/base.h

@@ -117,6 +117,7 @@ protected:
     std::unique_ptr<interfaces::Chain> m_chain;
     Chainstate* m_chainstate{nullptr};
     const std::string m_name;
+    const std::string m_thread_name;
 
     void BlockConnected(const kernel::ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override;
 
@@ -141,7 +142,7 @@ protected:
     void SetBestBlockIndex(const CBlockIndex* block);
 
 public:
-    BaseIndex(std::unique_ptr<interfaces::Chain> chain, std::string name);
+    BaseIndex(std::unique_ptr<interfaces::Chain> chain, std::string name, std::string thread_name);
     /// Destructor interrupts sync thread if running and blocks until it exits.
     virtual ~BaseIndex();
 

src/index/blockfilterindex.cpp

@@ -61,6 +61,15 @@ constexpr size_t CF_HEADERS_CACHE_MAX_SZ{2000};
 
 namespace {
 
+std::string BlockFilterThreadName(BlockFilterType filter_type)
+{
+    switch (filter_type) {
+    case BlockFilterType::BASIC: return "blkfltbscidx";
+    case BlockFilterType::INVALID: return "";
+    } // no default case, so the compiler can warn about missing cases
+    assert(false);
+}
+
 struct DBVal {
     uint256 hash;
     uint256 header;
@@ -75,7 +84,7 @@ static std::map<BlockFilterType, BlockFilterIndex> g_filter_indexes;
 
 BlockFilterIndex::BlockFilterIndex(std::unique_ptr<interfaces::Chain> chain, BlockFilterType filter_type,
                                    size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain), BlockFilterTypeName(filter_type) + " block filter index")
+    : BaseIndex(std::move(chain), BlockFilterTypeName(filter_type) + " block filter index", BlockFilterThreadName(filter_type))
     , m_filter_type(filter_type)
 {
     const std::string& filter_name = BlockFilterTypeName(filter_type);

src/index/coinstatsindex.cpp

@@ -87,7 +87,7 @@ struct DBVal {
 std::unique_ptr<CoinStatsIndex> g_coin_stats_index;
 
 CoinStatsIndex::CoinStatsIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain), "coinstatsindex")
+    : BaseIndex(std::move(chain), "coinstatsindex", "coinstatsidx")
 {
     // An earlier version of the index used "indexes/coinstats" but it contained
     // a bug and is superseded by a fixed version at "indexes/coinstatsindex".

src/index/txindex.cpp

@@ -66,7 +66,7 @@ void TxIndex::DB::WriteTxs(const std::vector<std::pair<Txid, CDiskTxPos>>& v_pos
 }
 
 TxIndex::TxIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain), "txindex"), m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe))
+    : BaseIndex(std::move(chain), "txindex", "txidx"), m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe))
 {}
 
 TxIndex::~TxIndex() = default;

src/index/txospenderindex.cpp

@@ -60,7 +60,7 @@ struct DBKey {
 };
 
 TxoSpenderIndex::TxoSpenderIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain), "txospenderindex"), m_db{std::make_unique<DB>(gArgs.GetDataDirNet() / "indexes" / "txospenderindex" / "db", n_cache_size, f_memory, f_wipe)}
+    : BaseIndex(std::move(chain), "txospenderindex", "txospenderidx"), m_db{std::make_unique<DB>(gArgs.GetDataDirNet() / "indexes" / "txospenderindex" / "db", n_cache_size, f_memory, f_wipe)}
 {
     if (!m_db->Read("siphash_key", m_siphash_key)) {
         FastRandomContext rng(false);

src/test/blockfilter_index_tests.cpp

@@ -338,9 +338,8 @@ private:
     int m_blocking_height;
 
 public:
-    explicit IndexReorgCrash(std::unique_ptr<interfaces::Chain> chain, std::shared_future<void> blocker,
-                             int blocking_height, FakeNodeClock& clock)
-        : BaseIndex(std::move(chain), "test index"), m_clock(clock), m_blocker(blocker), m_blocking_height(blocking_height)
+    explicit IndexReorgCrash(std::unique_ptr<interfaces::Chain> chain, std::shared_future<void> blocker, int blocking_height, FakeNodeClock& clock)
+        : BaseIndex(std::move(chain), "test index", "testidx"), m_clock(clock), m_blocker(blocker), m_blocking_height(blocking_height)
     {
         const fs::path path = gArgs.GetDataDirNet() / "index";
         fs::create_directories(path);

src/test/util_threadnames_tests.cpp

@@ -17,7 +17,7 @@ using util::ToString;
 
 BOOST_AUTO_TEST_SUITE(util_threadnames_tests)
 
-const std::string TEST_THREAD_NAME_BASE = "test_thread.";
+const std::string TEST_THREAD_NAME_BASE = "test_thrd.";
 
 /**
  * Run a bunch of threads to all call util::ThreadRename.
@@ -56,7 +56,7 @@ BOOST_AUTO_TEST_CASE(util_threadnames_test_rename_threaded)
 
     BOOST_CHECK_EQUAL(names.size(), 100U);
 
-    // Names "test_thread.[n]" should exist for n = [0, 99]
+    // Names "test_thrd.[n]" should exist for n = [0, 99]
     for (int i = 0; i < 100; ++i) {
         BOOST_CHECK(names.contains(TEST_THREAD_NAME_BASE + ToString(i)));
     }

src/util/threadnames.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <util/threadnames.h>
+#include <util/check.h>
 
 #include <algorithm>
 #include <cstring>
@@ -53,6 +54,7 @@ static void SetInternalName(const std::string& name)
 
 void util::ThreadRename(const std::string& name)
 {
+    Assume(name.size() <= 13); // Linux keeps 15 bytes
     SetThreadName(("b-" + name).c_str());
     SetInternalName(name);
 }

src/util/threadpool.h

@@ -112,7 +112,7 @@ public:
         // Create workers
         m_workers.reserve(num_workers);
         for (int i = 0; i < num_workers; i++) {
-            m_workers.emplace_back(&util::TraceThread, strprintf("%s_pool_%d", m_name, i), [this] { WorkerThread(); });
+            m_workers.emplace_back(&util::TraceThread, strprintf("%s.%02d", m_name, i), [this] { WorkerThread(); });
         }
     }
 

test/functional/feature_init.py

@@ -77,10 +77,10 @@ class InitTest(BitcoinTestFramework):
             b'net thread start',
             b'addcon thread start',
             b'initload thread start',
-            b'txindex thread start',
-            b'block filter index thread start',
-            b'coinstatsindex thread start',
-            b'txospenderindex thread start',
+            b'txidx thread start',
+            b'blkfltbscidx thread start',
+            b'coinstatsidx thread start',
+            b'txospenderidx thread start',
             b'msghand thread start',
             b'net thread start',
             b'addcon thread start',