util: shorten thread names to avoid Linux truncation #35173

nit: b-basicfltridx should IMO be renamed to include some version of "block".

    : BaseIndex(std::move(chain), BlockFilterTypeName(filter_type) + " block filter index", BlockFilterTypeName(filter_type).substr(0, 5) + "blkftrdb")

Giving b-basicblkftrdb (15 chars). Uses "DB" instead of "IDX" which is less accurate but shorter. 😬

Another strategy could be to consider dropping the b--prefix for Linux or all platforms since figuring out which process a thread belongs to shouldn't be too hard. It appears we already shortened the prefix once in the past though, see 386ae0f6916d11d72022cc6f6b62bb2b82594f24.

b-basicfltridx should IMO be renamed to include some version of "block".

It seems we have to drop something for it to fit. Maybe these other options might work: blkfltrbasic blkfltrbscidx

Another strategy could be to consider dropping the b--prefix

I find the prefix useful, as it makes the threads easily identifiable alongside others.

blkfltrbscidx

Converting "basic" into "bsc" would require implementing something like StripVowels() or adding another function than BlockFilterTypeName() (BlockFilterCrampedName()). Maybe not strictly necessary.

Maybe one could change the order to: b-blkfltidx<typename> and just let typename be truncated automatically.

(I think flt is better abbreviation for "filter", ftr feelds like "footer". Much bikeshedding :grin:).

I find the prefix useful, as it makes the threads easily identifiable alongside others.

Yeah, I guess it makes it easier to differentiate our threads from those spawned by libraries we depend on.

Thanks for the comments, changed to blkfltbscidx

In 8924385d370068d755a57056c2d3a09f8cd59821 "util: shorten ThreadPool worker names"

Onboard with the new pool name but it's quite similar to the above http thread that listens for the connections, specially differentiating between b-http and b-http.0 can be confusing for the uninitiated.

Maybe s/b-http/b-http-server (which is less than 15 chars)? But it might not go with the aim of the PR of shortening the names.

Thanks for taking a look! I don't have a strong preference either way, but I'd lean toward minimal diff here. The .N suffix is a uniform convention for all ThreadPool workers (e.g. PR #31132 introduces b-inputfetch.N for parallel input fetching, and any future pool will likely follow the same pattern). The dot is a deliberate visual cue: any b-<name>.N is a pool worker, anything without a dot is a standalone thread. Special-casing HTTP (either by renaming the listener or by giving its pool a different suffix scheme) would probably weaken that consistent pattern just as it's being established.

What do other reviewers think?

The dot is a deliberate visual cue: any b-<name>.N is a pool worker, anything without a dot is a standalone thread.

I'm onboard with this new naming for the pool workers because of the truncation issue. While the dot acts as a visual cue, it's not as strong of a cue as the earlier _pool_ was. In order to clearly differentiate the pool threads (http.0 specifically) from the listener thread, this PR can consider updating the bare b-http to b-httplistener or similar - the httplistener portion is the <name>.

But I should also mention that this is not a strong opinion from my side - will leave it to your preference.

Thanks, I’m leaning toward leaving the listener name unchanged for this PR because the listener itself does not have a truncation problem, and renaming it would widen the scope beyond the ThreadPool worker suffix change. The PR description now calls out the dotted suffix as the uniform worker convention. If you or other reviewers feel strongly about it I don't mind extending the PR.

Is there a value in providing a default here? Is this a leftover from over-splitting the commit? Maybe all of the index commits could be squashed into one?

Also, I wonder if for the index threads it would be cleaner to use the b-i-$short_name prefix: b-i-tx, b-i-txospender. I presume that devs are familiar with the meaning of i being an index. But just my opinion, not sure what others think.

Maybe all of the index commits could be squashed into one?

I would prefer this.

Is there a value in providing a default here?

No, not anymore. I made all BaseIndex call sites pass the thread name explicitly.

Maybe all of the index commits could be squashed into one?

Agreed. I squashed the indexer thread-name changes into one commit now that the names are explicit instead of being derived incrementally.

Also, I wonder if for the index threads it would be cleaner to use the b-i-$short_name prefix: b-i-tx, b-i-txospender

I don't have a strong objection. For now I kept the idx-suffixed names because they fit under the b- limit and already had review support, but happy to switch if reviewers prefer the b-i-* convention.

Also fixed the stale Doxygen link in doc/developer-notes.md in the latest push and added some clarifying rewording for #35173 (review)

I made all BaseIndex call sites pass the thread name explicitly.

much better

For now I kept the idx-suffixed names because they fit under the b- limit

IMO, the idx-suffixed names are more immediately obvious at a glance than the b-i-* convention.

nanonit: Would prefer prepending 0 to single digits so that thread names sort in order:

            m_workers.emplace_back(&util::TraceThread, strprintf("%s.%02d", m_name, i), [this] { WorkerThread(); });

Same could be done for:

--- a/src/checkqueue.h
+++ b/src/checkqueue.h
@@ -148,7 +148,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 */);
             });
         }

I tried to find places where this actually helped but htop and Sublime Debugger already seem to sort them correctly. Maybe the default is to sort by thread id as a higher priority than names.

Initializing all BlockFilterIndex with the same thread name is incorrect. Knots has/had a second index to filter transactions based off segwit/non-segwit I think. There's also recently been talk of adding a new index with less compute but higher bandwidth if I recall correctly.

<details><summary>Possible fix diff</summary>

diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp
index 44d08440d1..e71b1af401 100644
--- a/src/blockfilter.cpp
+++ b/src/blockfilter.cpp
@@ -19,8 +19,12 @@
 
 using util::Join;
 
-static const std::map<BlockFilterType, std::string> g_filter_types = {
-    {BlockFilterType::BASIC, "basic"},
+struct FilterEntry {
+    BlockFilterType type;
+    std::string name;
+    std::string thread_name;
+} static constexpr g_filter_types[] = {
+    {BlockFilterType::BASIC, "basic", "blkfltbscidx"},
 };
 
 uint64_t GCSFilter::HashToRange(const Element& element) const
@@ -147,16 +151,27 @@ bool GCSFilter::MatchAny(const ElementSet& elements) const
 
 const std::string& BlockFilterTypeName(BlockFilterType filter_type)
 {
+    for (const auto& entry : g_filter_types) {
+        if (entry.type == filter_type) return entry.name;
+    }
+    static std::string unknown_retval;
+    return unknown_retval;
+}
+
+const std::string& BlockFilterThreadName(BlockFilterType filter_type)
+{
+    for (const auto& entry : g_filter_types) {
+        if (entry.type == filter_type) return entry.thread_name;
+    }
     static std::string unknown_retval;
-    auto it = g_filter_types.find(filter_type);
-    return it != g_filter_types.end() ? it->second : unknown_retval;
+    return unknown_retval;
 }
 
 bool BlockFilterTypeByName(std::string_view name, BlockFilterType& filter_type)
 {
     for (const auto& entry : g_filter_types) {
-        if (entry.second == name) {
-            filter_type = entry.first;
+        if (entry.name == name) {
+            filter_type = entry.type;
             return true;
         }
     }
@@ -170,7 +185,7 @@ const std::set<BlockFilterType>& AllBlockFilterTypes()
     static std::once_flag flag;
     std::call_once(flag, []() {
             for (const auto& entry : g_filter_types) {
-                types.insert(entry.first);
+                types.insert(entry.type);
             }
         });
 
@@ -179,7 +194,7 @@ const std::set<BlockFilterType>& AllBlockFilterTypes()
 
 const std::string& ListBlockFilterTypes()
 {
-    static std::string type_list{Join(g_filter_types, ", ", [](const auto& entry) { return entry.second; })};
+    static std::string type_list{Join(std::span{g_filter_types}, ", ", [](const auto& entry) { return entry.name; })};
 
     return type_list;
 }
diff --git a/src/blockfilter.h b/src/blockfilter.h
index 225d3b16be..40d24a725d 100644
--- a/src/blockfilter.h
+++ b/src/blockfilter.h
@@ -99,6 +99,9 @@ enum class BlockFilterType : uint8_t
 /** Get the human-readable name for a filter type. Returns empty string for unknown types. */
 const std::string& BlockFilterTypeName(BlockFilterType filter_type);
 
+/** Get the short thread name for the filter index thread. */
+const std::string& BlockFilterThreadName(BlockFilterType filter_type);
+
 /** Find a filter type by its human-readable name. */
 bool BlockFilterTypeByName(std::string_view name, BlockFilterType& filter_type);
 
diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp
index 0f69f99902..092889bbaf 100644
--- a/src/index/blockfilterindex.cpp
+++ b/src/index/blockfilterindex.cpp
@@ -75,7 +75,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", "blkfltbscidx")
+    : 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);

</details>