diff --git a/src/gxs/rsdataservice.cc b/src/gxs/rsdataservice.cc index 022704740..472e5cf0b 100644 --- a/src/gxs/rsdataservice.cc +++ b/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 &msgs, int metaOffset) +void RsDataService::locked_retrieveMessages(RetroCursor *c, std::vector &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)); diff --git a/src/gxs/rsdataservice.h b/src/gxs/rsdataservice.h index 3fd2fb748..e136dc042 100644 --- a/src/gxs/rsdataservice.h +++ b/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& 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& msgs, int metaOffset, + const RsGxsGroupId* expected_grp = nullptr); /*! * Retrieves all the grp results from a cursor diff --git a/src/services/p3gxschannels.cc b/src/services/p3gxschannels.cc index 39001b211..c8fb8d588 100644 --- a/src/services/p3gxschannels.cc +++ b/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({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"