GXS: stop the batched retrieval from rescanning the group once per batch

Profiling the version filtering added in the previous commit showed the
batched IN(...) retrieval costing a near constant 60 to 95ms per batch
whatever the number of messages it returned: 5 batches for 2203 messages took
454ms, 7 batches for 3127 took 451ms. That is the signature of each batch
walking the whole group, and it made a channel with almost no edited posts
slower than before (272ms in one query against 451ms in seven).

The cause is the query plan. On "grpId=... AND msgId IN (...)" sqlite has no
ANALYZE data, so it estimates an equality on the non unique group index at
about ten rows, against one row per entry of the IN list. The group therefore
looks fifty times more selective than it really is -- it matches every message
of the channel -- and gets picked, so every batch scans the group and filters.

Select on the message id alone. It is the table's primary key, hence unique
table wide, so the result is identical while sqlite can seek straight into the
implicit unique index. locked_retrieveMessages() takes an optional expected
group and drops anything else, so a caller mixing groups still cannot get
foreign messages attributed to the wrong one.

Reading most of a group by id remains slower than scanning it once, whatever
the index. So getChannelAllContent() now only filters versions out when they
are worth filtering: past MAX_READ_RATIO_FOR_VERSION_FILTERING of the group's
messages it requests the whole group and resolves versions with sortPosts() as
before. The profiling line reports which path was taken.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/gxs/rsdataservice.cc

@@ -1215,9 +1215,21 @@ int RsDataService::retrieveNxsMsgs(const GxsMsgReq &reqIds, GxsMsgResult &msg,
             // message dominates the cost as soon as a request covers more than
             // a handful of them (a channel post and its comments, a forum
             // thread, a filtered group request).
+            //
+            // The group is deliberately left out of the selection. Adding
+            // "grpId=..." makes sqlite pick INDEX_MESSAGES_GRPID over the
+            // implicit unique index on the message id: with no ANALYZE data it
+            // estimates an equality on a non unique index at ~10 rows, against
+            // one row per entry of the IN list, so the group looks far more
+            // selective than it is. Every batch then walks the whole group and
+            // filters, which measured as a constant ~60-95ms per batch no
+            // matter how few messages it returned. Selecting on the message id
+            // alone keeps the cost proportional to the number of ids asked for.
+            // Message ids are unique table wide, so the result is the same;
+            // locked_retrieveMessages() still drops anything from another group
+            // in case a caller mixes them up.
 
-            const std::string selection_prefix = KEY_GRP_ID + "='" + grpId.toStdString()
-                                               + "' AND " + KEY_MSG_ID + " IN (";
+            const std::string selection_prefix = KEY_MSG_ID + " IN (";
 
             for(auto sit = msgIdV.begin(); sit != msgIdV.end(); )
             {
@@ -1236,7 +1248,7 @@ int RsDataService::retrieveNxsMsgs(const GxsMsgReq &reqIds, GxsMsgResult &msg,
 
                 if(c)
                 {
-                    locked_retrieveMessages(c, msgSet, withMeta ? mColMsg_WithMetaOffset : 0);
+                    locked_retrieveMessages(c, msgSet, withMeta ? mColMsg_WithMetaOffset : 0, &grpId);
                 }
 
                 delete c;
@@ -1274,12 +1286,19 @@ int RsDataService::retrieveNxsMsgs(const GxsMsgReq &reqIds, GxsMsgResult &msg,
     return 1;
 }
 
-void RsDataService::locked_retrieveMessages(RetroCursor *c, std::vector<RsNxsMsg *> &msgs, int metaOffset)
+void RsDataService::locked_retrieveMessages(RetroCursor *c, std::vector<RsNxsMsg *> &msgs, int metaOffset,
+                                            const RsGxsGroupId* expected_grp)
 {
     bool valid = c->moveToFirst();
     while(valid){
         RsNxsMsg* m = locked_getMessage(*c);
 
+        if(m && expected_grp && m->grpId != *expected_grp)
+        {
+            delete m;
+            m = nullptr;
+        }
+
         if(m){
             if (metaOffset)
                 m->metaData = new RsGxsMsgMetaData(*locked_getMsgMeta(*c, metaOffset));

src/gxs/rsdataservice.h

@@ -283,7 +283,13 @@ private:
      * @param c cursor to result set
      * @param msgs messages retrieved from cursor are stored here
      */
-    void locked_retrieveMessages(RetroCursor* c, std::vector<RsNxsMsg*>& msgs, int metaOffset);
+    /*!
+     * @param expected_grp when not null, messages belonging to another group are
+     *        discarded. Used by the id based retrieval, whose query selects on
+     *        the message id alone.
+     */
+    void locked_retrieveMessages(RetroCursor* c, std::vector<RsNxsMsg*>& msgs, int metaOffset,
+                                 const RsGxsGroupId* expected_grp = nullptr);
 
     /*!
      * Retrieves all the grp results from a cursor

src/services/p3gxschannels.cc

@@ -139,6 +139,12 @@ uint32_t p3GxsChannels::channelsAuthenPolicy()
 	return policy;
 }
 
+/** Above this share of a channel's messages, reading them by explicit id costs
+ * more than scanning the group once, so getChannelAllContent() stops filtering
+ * out superseded post versions and falls back to reading everything. Tune with
+ * the RS_GXS_PROFILE output of getChannelAllContent(). */
+static const uint32_t MAX_READ_RATIO_FOR_VERSION_FILTERING = 66; // percent
+
 static const uint32_t GXS_CHANNELS_CONFIG_MAX_TIME_NOTIFY_STORAGE = 86400*30*2 ; // ignore notifications for 2 months
 static const uint8_t  GXS_CHANNELS_CONFIG_SUBTYPE_NOTIFY_RECORD   = 0x01 ;
 
@@ -1613,14 +1619,29 @@ bool p3GxsChannels::getChannelAllContent( const RsGxsGroupId& channelId,
     if(wanted_msgs.empty())
         return true;
 
+    // Skipping versions only pays off when there are enough of them to skip.
+    // Below that, asking for an explicit id list costs more than it saves: the
+    // database walks the group anyway, and reading most of a group by id is
+    // slower than scanning it once. In that case take the whole group and let
+    // sortPosts() resolve the versions as it always did.
+
+    const bool worth_filtering =
+            wanted_msgs.size() * 100 <= metas.size() * MAX_READ_RATIO_FOR_VERSION_FILTERING;
+
     uint32_t token;
     RsTokReqOptions opts;
     opts.mReqType = GXS_REQUEST_TYPE_MSG_DATA;
 
-    GxsMsgReq msgIds;
-    msgIds[channelId] = wanted_msgs;
+    if(worth_filtering)
+    {
+        GxsMsgReq msgIds;
+        msgIds[channelId] = wanted_msgs;
 
-    if( !requestMsgInfo(token, opts, msgIds) || waitToken(token,std::chrono::milliseconds(60000)) != RsTokenService::COMPLETE )
+        if( !requestMsgInfo(token, opts, msgIds) || waitToken(token,std::chrono::milliseconds(60000)) != RsTokenService::COMPLETE )
+            return false;
+    }
+    else if( !requestMsgInfo(token, opts, std::list<RsGxsGroupId>({channelId}))
+             || waitToken(token,std::chrono::milliseconds(60000)) != RsTokenService::COMPLETE )
         return false;
 
     const long prof_wait_ms = prof_timer.lap();
@@ -1630,15 +1651,19 @@ bool p3GxsChannels::getChannelAllContent( const RsGxsGroupId& channelId,
     if(!convertMsgItems(token, posts, comments, votes, prof_getmsgdata_ms, prof_convert_ms))
         return false;
 
-    applyPostVersions(posts, comments, retained, version_to_latest);
+    if(worth_filtering)
+        applyPostVersions(posts, comments, retained, version_to_latest);
+    else
+        sortPosts(posts, comments);
 
     const long prof_read_ms = prof_timer.ms();
     const long prof_total_ms = prof_versions_ms + prof_wait_ms + prof_read_ms;
 
     RS_GXS_PROF( prof_total_ms, "getChannelAllContent grp=" << channelId
                  << " metas=" << metas.size()
-                 << " read_msgs=" << wanted_msgs.size()
-                 << " skipped_versions=" << (metas.size() - wanted_msgs.size())
+                 << " read_msgs=" << (worth_filtering ? wanted_msgs.size() : metas.size())
+                 << " skipped_versions=" << (worth_filtering ? (metas.size() - wanted_msgs.size()) : 0)
+                 << " filtered=" << (worth_filtering ? "yes" : "no")
                  << " versions=" << prof_versions_ms << "ms"
                  << " token_wait=" << prof_wait_ms << "ms"
                  << " read=" << prof_read_ms << "ms"