From de922d7e8661f8ea2ec1128a59290b0f3730a85d Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 15:22:19 -0600 Subject: [PATCH 1/2] perf(tags): no assignment lookup for tables that have no tags Core selects the Tags virtual field for every query or get that names no fields, so every such read of a TAGS-enabled table ran one collaboration_record_tag statement, including reads made one record at a time and tables that were never tagged. Measured with 600 added parents (TagLookupStatementBudgetTest): a 602-row list ran 2 statements, 600 gets ran 600 and 100 single-record updates ran 100; now 1 probe, then 0. TaggedTables remembers, per node and per table, whether any assignment exists (core Memoization registered with ClusterCaches, 30 s by default, withTagLookupCacheSeconds, 0 disables). Writing an assignment clears it at once and after commit and publishes memo-invalidate. The probe ignores row locks and returns no data, so the answer is the same for every session. Tables with assignments are read as before: one lookup per 500 parents. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 1 + README.md | 9 + UPGRADE.md | 1 + .../CollaborationQBitConfig.java | 23 ++ .../collaboration/CollaborationRealtime.java | 11 +- .../collaboration/tags/TagValueProvider.java | 16 +- .../collaboration/tags/TaggedTables.java | 184 ++++++++++ .../TagLookupStatementBudgetTest.java | 338 ++++++++++++++++++ 8 files changed, 579 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/kingsrook/qbits/collaboration/tags/TaggedTables.java create mode 100644 src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 7541022..ce9a9a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ All notable changes are documented here, following [Keep a Changelog](https://ke ### Fixed +- Reads of a table that has no tag assignment no longer query `collaboration_record_tag`. Core selects the Tags field for every query or get that names no fields, so each of them (including per-record gets and the stored-record read before an update) ran one assignment statement whether or not the table had ever been tagged: 600 gets of an untagged table ran 600 statements, now none. Each node remembers which tables have no assignment (`TaggedTables`, core `Memoization` registered with `ClusterCaches`) for `CollaborationQBitConfig.withTagLookupCacheSeconds` (default 30, 0 disables); writing an assignment clears it on the node and through the ClusterBus. Tables with assignments are read as before, one lookup per 500 parents. No API or schema change. See "What the Tags field costs a read" in the README. - The retained upload journal's PostgreSQL 17 qualification accepts Amazon Aurora PostgreSQL, which reports `full_page_writes=off` and does not let a deployment change it. `off` is accepted only when the settings query detects Aurora (`to_regprocedure('aurora_version()') IS NOT NULL`); stock PostgreSQL with `full_page_writes=off` is still refused, and every other requirement is unchanged on Aurora. See `docs/RETAINED-UPLOAD-JOURNAL.md`. - Bounded STRING custom fields carry QQQ's native `ValueTooLongBehavior.ERROR`, so a write longer than `maxLength` is rejected with a field error and the stored value is preserved, instead of passing through unchecked (the QQQ default `PASS_THROUGH`). - Persisted help content is reloaded after each committed runtime metadata refresh, so sections and fields created by publishing a custom-field definition receive their stored `helpContent` rows without a restart. diff --git a/README.md b/README.md index ec0266f..b39767f 100644 --- a/README.md +++ b/README.md @@ -411,6 +411,15 @@ size, pending suggestions, and the five most-used approved tags. The runtime provider adds this field and its search entry where TAGS is enabled. It is queryable but not selectable: the overlay has no virtual-field post-query hook and existing host customizers must be preserved. +### What the Tags field costs a read + +Core selects every selectable virtual field for a query or get that names no fields, so each such read of a table with TAGS enabled runs `TagValueProvider`, including the reads other code makes one record at a time (a get, the stored-record read before an update, a lookup made per row). The provider therefore costs: + +- nothing for a table that has no tag assignment. Each node remembers that one fact per table (`TaggedTables`) for `withTagLookupCacheSeconds` (default 30; 0 looks every time) and learns it with one indexed `LIMIT 1` statement; +- one assignment statement and one vocabulary statement per 500 parents for a table that has assignments. Record tags themselves are never cached. + +A node that writes an assignment forgets the table's answer at once and again after commit, and publishes core's `memo-invalidate` cluster event (`ClusterCaches`, name `collaboration.taggedTables`). Hosts with a ClusterBus transport see a table's first tag on every node immediately; without one, another node shows it after at most the configured seconds. The same bound covers assignments written outside QQQ. The answer does not depend on the session (the probe ignores row locks and returns no data), so it cannot change which records or tags a caller sees. A failed probe is thrown, not remembered. + ## UI API The producer registers extension `collaboration` version `1.2.0` at diff --git a/UPGRADE.md b/UPGRADE.md index 815b944..e5937b0 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -5,4 +5,5 @@ No schema change and nothing required of hosts. - New table action **Create rows from configuration** on Feature Settings. Run it once if you want the effective feature settings visible and editable as rows. It changes no behavior and does nothing once the table has any row. Note that after rows exist they win over later changes to `withDefaultFeatures` or `withTableFeatures` in code, so change those settings in the rows from then on. +- Reads of tables without any tag assignment skip the assignment lookup (`withTagLookupCacheSeconds`, default 30 seconds, 0 restores a lookup on every read). A host running several nodes without a ClusterBus transport shows a table's very first tag on the other nodes after at most that long; later tags on that table show at once, as before. - New **Setup** widget first on the Collaboration home, and table help text. Hosts that override help on a table keep their own. diff --git a/src/main/java/com/kingsrook/qbits/collaboration/CollaborationQBitConfig.java b/src/main/java/com/kingsrook/qbits/collaboration/CollaborationQBitConfig.java index 78c79ed..39768ab 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/CollaborationQBitConfig.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/CollaborationQBitConfig.java @@ -51,6 +51,7 @@ public class CollaborationQBitConfig implements QBitConfig private Function recipientEmailResolver = null; private QCodeReference userSessionProvider = null; private String defaultTagMode = "FREE"; + private Integer tagLookupCacheSeconds = 30; private List tagActionTables = List.of(); private List followActionTables = List.of(); private List followChangeTables = List.of(); @@ -325,6 +326,7 @@ public class CollaborationQBitConfig implements QBitConfig assertCondition(instance.getBackend(backendName) != null, "Unknown Collaboration backend: " + backendName, errors); } assertCondition(List.of("FREE", "APPROVED_ONLY").contains(defaultTagMode == null ? "" : defaultTagMode), "defaultTagMode must be FREE or APPROVED_ONLY", errors); + assertCondition(tagLookupCacheSeconds != null && tagLookupCacheSeconds >= 0, "tagLookupCacheSeconds must be 0 (off) or more", errors); for(List tables : List.of(followActionTables, followChangeTables, tagActionTables, assignActionTables)) { for(String table : tables) @@ -550,6 +552,27 @@ public class CollaborationQBitConfig implements QBitConfig return (this); } + /******************************************************************************* + ** How long a node remembers whether a table has any tag assignment. + *******************************************************************************/ + public Integer getTagLookupCacheSeconds() + { + return (tagLookupCacheSeconds); + } + + /******************************************************************************* + ** Reads of a table without any tag assignment skip the assignment lookup while + ** this node remembers that (default 30 seconds; 0 looks every time). A node + ** that writes an assignment forgets at once and tells the others over the + ** ClusterBus; without a cluster transport another node shows a table's first + ** tag after at most this long. Tables that have assignments are always read. + *******************************************************************************/ + public CollaborationQBitConfig withTagLookupCacheSeconds(Integer tagLookupCacheSeconds) + { + this.tagLookupCacheSeconds = tagLookupCacheSeconds; + return (this); + } + /******************************************************************************* ** Host tables receiving Add tags and Remove tags actions. *******************************************************************************/ diff --git a/src/main/java/com/kingsrook/qbits/collaboration/CollaborationRealtime.java b/src/main/java/com/kingsrook/qbits/collaboration/CollaborationRealtime.java index 4c7c5cf..6734370 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/CollaborationRealtime.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/CollaborationRealtime.java @@ -33,7 +33,8 @@ import com.kingsrook.qqq.backend.core.realtime.RealtimeBus; import com.kingsrook.qqq.backend.core.realtime.RealtimeMessage; /******************************************************************************* - ** Identifier-only hints, published after successful commit. + ** Identifier-only hints, published after successful commit. A tag assignment + ** also clears what this node remembers about untagged tables (TaggedTables). *******************************************************************************/ public class CollaborationRealtime implements RecordChangeListenerInterface { @@ -55,6 +56,7 @@ public class CollaborationRealtime implements RecordChangeListenerInterface public void onRecordsChanged(RecordChangeEvent change) throws QException { var instance = QContext.getQInstance(); + java.util.Set newlyTaggedTables = new java.util.HashSet<>(); for(int index = 0; index < change.getRecords().size(); index++) { var row = new com.kingsrook.qqq.backend.core.model.data.QRecord(change.getRecords().get(index)); @@ -70,6 +72,13 @@ public class CollaborationRealtime implements RecordChangeListenerInterface { continue; } + if(CollaborationTables.RECORD_TAG.equals(change.getTableName()) && change.getType() != RecordChangeType.DELETE && newlyTaggedTables.add(table)) + { + //////////////////////////////////////////////////////////////////////// + // the Tags field skips tables known to have no assignment: forget it // + //////////////////////////////////////////////////////////////////////// + com.kingsrook.qbits.collaboration.tags.TaggedTables.assignmentsChanged(table, change.getTransaction()); + } String event = switch(change.getTableName()) { case CollaborationTables.COMMENT -> change.getType() == RecordChangeType.DELETE || row.getValue("deletedAt") != null diff --git a/src/main/java/com/kingsrook/qbits/collaboration/tags/TagValueProvider.java b/src/main/java/com/kingsrook/qbits/collaboration/tags/TagValueProvider.java index 4bccd84..7703c28 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/tags/TagValueProvider.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/tags/TagValueProvider.java @@ -45,16 +45,26 @@ public class TagValueProvider implements VirtualFieldValueProviderInterface { /******************************************************************************* ** Batch both assignments and vocabulary; never recursively load parent records. + ** A table without any assignment costs no statement: core selects this field + ** for every read that names no fields, so untagged tables must stay free. *******************************************************************************/ @Override public void fill(QTableMetaData table, String fieldName, List records, QueryInput origin) throws QException { + if(!TaggedTables.mayHaveAssignments(table.getName(), origin.getTransaction())) + { + for(QRecord record : records) + { + record.setValue(fieldName, null); + } + return; + } + QTableMetaData metadata = QContext.getQInstance().getTable(CollaborationTables.RECORD_TAG).clone(); + metadata.withCustomizers(null); + metadata.getSupplementalMetaData().remove(RecordReferenceTableMetaData.NAME); for(List page : CollectionUtils.getPages(records, 500)) { List ids = page.stream().map(record -> String.valueOf(record.getValue(table.getPrimaryKeyField()))).distinct().toList(); - QTableMetaData metadata = QContext.getQInstance().getTable(CollaborationTables.RECORD_TAG).clone(); - metadata.withCustomizers(null); - metadata.getSupplementalMetaData().remove(RecordReferenceTableMetaData.NAME); QueryInput input = new QueryInput(CollaborationTables.RECORD_TAG).withTransaction(origin.getTransaction()).withFilter(new QQueryFilter() .withCriteria("tableName", QCriteriaOperator.EQUALS, table.getName()).withCriteria("recordId", QCriteriaOperator.IN, ids) .withCriteria("parentDeleted", QCriteriaOperator.EQUALS, false)); diff --git a/src/main/java/com/kingsrook/qbits/collaboration/tags/TaggedTables.java b/src/main/java/com/kingsrook/qbits/collaboration/tags/TaggedTables.java new file mode 100644 index 0000000..81c2daa --- /dev/null +++ b/src/main/java/com/kingsrook/qbits/collaboration/tags/TaggedTables.java @@ -0,0 +1,184 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2025. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +package com.kingsrook.qbits.collaboration.tags; + +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.WeakHashMap; +import com.kingsrook.qbits.collaboration.CollaborationRecords; +import com.kingsrook.qbits.collaboration.model.CollaborationTables; +import com.kingsrook.qqq.backend.core.actions.QBackendTransaction; +import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; +import com.kingsrook.qqq.backend.core.cluster.ClusterBus; +import com.kingsrook.qqq.backend.core.cluster.ClusterCaches; +import com.kingsrook.qqq.backend.core.cluster.ClusterEvent; +import com.kingsrook.qqq.backend.core.context.QContext; +import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QCriteriaOperator; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; +import com.kingsrook.qqq.backend.core.model.metadata.QInstance; +import com.kingsrook.qqq.backend.core.utils.memoization.Memoization; + +/******************************************************************************* + ** Which host tables have any tag assignment at all. Every table with TAGS + ** enabled carries the Tags field, so every read of such a table would otherwise + ** look for assignments, including the tables nobody ever tagged. + ** + ** Only that one fact per table is remembered, never a record's tags: a table + ** known to have assignments is always read from the database. The answer is the + ** same for every session (the probe ignores row locks and returns no data), so + ** it cannot widen or narrow what a caller sees. + ** + ** Each node keeps its own answers. An assignment written here clears the + ** table's answer on this node at once and again after commit, and on the other + ** nodes through the ClusterBus (memo-invalidate). The timeout bounds everything + ** that event cannot reach: hosts without a cluster transport, writes made + ** outside QQQ, and a probe that raced an uncommitted assignment. + *******************************************************************************/ +public final class TaggedTables +{ + public static final String CACHE_NAME = "collaboration.taggedTables"; + + private static final int MAX_TABLES = 10_000; + + private static final Map> CACHES = ClusterCaches.shared(CACHE_NAME, + new WeakHashMap>(), TaggedTables::clearAll, TaggedTables::clearTable); + + + + /******************************************************************************* + ** Static lookups only. + *******************************************************************************/ + private TaggedTables() + { + } + + + + /******************************************************************************* + ** False only when the table is known to have no assignment. A failed probe is + ** thrown, never remembered. With the cache disabled the answer is always true. + *******************************************************************************/ + public static boolean mayHaveAssignments(String tableName, QBackendTransaction transaction) throws QException + { + Integer seconds = CollaborationRecords.config().getTagLookupCacheSeconds(); + if(seconds == null || seconds <= 0) + { + return (true); + } + return (cache(QContext.getQInstance(), seconds).getResultThrowing(tableName, name -> hasAssignments(name, transaction)).orElse(true)); + } + + + + /******************************************************************************* + ** An assignment was written for this parent table. Forget the answer now, so + ** the writing transaction reads its own assignment, and again after commit, so + ** a probe that ran in between and saw nothing cannot outlive the commit. + *******************************************************************************/ + public static void assignmentsChanged(String tableName, QBackendTransaction transaction) + { + QInstance instance = QContext.getQInstance(); + forget(instance, tableName); + if(transaction != null) + { + transaction.addAfterCommitCallback(() -> forget(instance, tableName)); + } + } + + + + /******************************************************************************* + ** Clear on this node and tell the others; a transport failure is logged by the + ** bus and the timeout covers the nodes that did not hear. + *******************************************************************************/ + private static void forget(QInstance instance, String tableName) + { + ClusterBus.publish(instance, new ClusterEvent(ClusterEvent.TYPE_MEMO_INVALIDATE, CACHE_NAME, tableName)); + } + + + + /******************************************************************************* + ** One indexed row at most, in the caller's transaction, as the same answer for + ** every session: no row locks, no parent check and no customizers. + *******************************************************************************/ + private static Boolean hasAssignments(String tableName, QBackendTransaction transaction) throws QException + { + QueryInput input = new QueryInput(CollaborationTables.RECORD_TAG).withTransaction(transaction).withFieldNamesToInclude(Set.of("id")) + .withFilter(new QQueryFilter().withCriteria("tableName", QCriteriaOperator.EQUALS, tableName).withLimit(1)); + input.setTableMetaData(CollaborationRecords.cleanupMetadata(CollaborationTables.RECORD_TAG)); + input.withPermissionBypass("Tag provider asks only whether a table has any assignment."); + return (!new QueryAction().execute(input).getRecords().isEmpty()); + } + + + + /******************************************************************************* + ** The instance's answers, following the configured timeout. + *******************************************************************************/ + private static Memoization cache(QInstance instance, int seconds) + { + synchronized(CACHES) + { + Memoization cache = CACHES.computeIfAbsent(instance, key -> new Memoization(MAX_TABLES).withMayStoreNullValues(false)); + cache.setTimeout(Duration.ofSeconds(seconds)); + return (cache); + } + } + + + + /******************************************************************************* + ** Every instance in this JVM: an extra probe is cheaper than a stale answer. + *******************************************************************************/ + private static List> caches() + { + synchronized(CACHES) + { + return (List.copyOf(CACHES.values())); + } + } + + + + /******************************************************************************* + ** ClusterCaches: forget every table. + *******************************************************************************/ + private static void clearAll() + { + caches().forEach(Memoization::clear); + } + + + + /******************************************************************************* + ** ClusterCaches: forget one table. + *******************************************************************************/ + private static void clearTable(String tableName) + { + caches().forEach(cache -> cache.clearKey(tableName)); + } +} diff --git a/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java b/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java new file mode 100644 index 0000000..5819d4d --- /dev/null +++ b/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java @@ -0,0 +1,338 @@ +/* + * QQQ - Low-code Application Framework for Engineers. + * Copyright (C) 2021-2025. Kingsrook, LLC + * 651 N Broad St Ste 205 # 6917 | Middletown DE 19709 | United States + * contact@kingsrook.com + * https://github.com/Kingsrook/ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +package com.kingsrook.qbits.collaboration; + + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import com.kingsrook.qbits.collaboration.model.CollaborationTables; +import com.kingsrook.qbits.collaboration.tags.TagEntry; +import com.kingsrook.qbits.collaboration.tags.TaggedTables; +import com.kingsrook.qbits.collaboration.tags.TagsService; +import com.kingsrook.qqq.backend.core.actions.QBackendTransaction; +import com.kingsrook.qqq.backend.core.actions.collaboration.RuntimeMetaDataOverlay; +import com.kingsrook.qqq.backend.core.actions.tables.GetAction; +import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; +import com.kingsrook.qqq.backend.core.actions.tables.UpdateAction; +import com.kingsrook.qqq.backend.core.cluster.ClusterCacheAudit; +import com.kingsrook.qqq.backend.core.cluster.ClusterCaches; +import com.kingsrook.qqq.backend.core.context.QContext; +import com.kingsrook.qqq.backend.core.model.actions.AbstractActionInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.get.GetInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.insert.InsertInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QQueryFilter; +import com.kingsrook.qqq.backend.core.model.actions.tables.query.QueryInput; +import com.kingsrook.qqq.backend.core.model.actions.tables.update.UpdateInput; +import com.kingsrook.qqq.backend.core.model.data.QRecord; +import com.kingsrook.qqq.backend.core.model.metadata.collaboration.RecordReferenceSecurityConfig; +import com.kingsrook.qqq.backend.core.modules.backend.implementations.memory.MemoryRecordStore; +import com.kingsrook.qqq.backend.core.utils.lambdas.UnsafeVoidVoidMethod; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; + + +/******************************************************************************* + ** Count the backend queries this qbit adds to ordinary reads of a host table. + ** Every backend query is one SQL statement (and one pool checkout) on an RDBMS + ** backend, and core selects the Tags field for every read that names no fields: + ** a table nobody tagged must not pay for it, and a tagged table pays per page. + ** + ** Before TaggedTables (1.2.0-RC.2), with 600 added parents: a 602-row list ran + ** 2 assignment queries, 600 gets ran 600 and 100 single-record updates ran 100. + *******************************************************************************/ +class TagLookupStatementBudgetTest extends PersonalServiceFixture +{ + private static final int PARENTS = 600; + + private final TagsService tags = new TagsService(); + + + + /******************************************************************************* + ** 600 more visible parents, and the Tags field every enabled table receives. + *******************************************************************************/ + @BeforeEach + void parents() throws Exception + { + List parents = new ArrayList<>(); + for(int id = 4; id <= PARENTS + 3; id++) + { + parents.add(new QRecord().withValue("id", id).withValue("title", "Document " + id).withValue("tenant", "a")); + } + MemoryRecordStore.getInstance().insert(new InsertInput("document").withRecords(parents), true); + RuntimeMetaDataOverlay.refresh(instance); + assertThat(instance.getTable("document").getVirtualFields()).containsKey("collaborationTags"); + } + + + + /******************************************************************************* + ** A table nobody tagged: one probe, then lists, per-record gets and per-record + ** updates (which read the stored record first) add nothing at all. + *******************************************************************************/ + @Test + void untaggedTableCostsOneProbeThenNothing() throws Exception + { + assertThat(collaborationQueries(this::listWithoutTags)).isEqualTo(Map.of(CollaborationTables.RECORD_TAG, 1L)); + assertThat(collaborationQueries(this::listWithoutTags)).isEmpty(); + assertThat(collaborationQueries(() -> + { + for(int id = 4; id <= PARENTS + 3; id++) + { + assertThat(new GetAction().executeForRecord(new GetInput("document").withPrimaryKey(id)).getValue("collaborationTags")).isNull(); + } + })).isEmpty(); + assertThat(collaborationQueries(() -> + { + for(int id = 4; id <= 103; id++) + { + new UpdateAction().execute(new UpdateInput("document").withRecords(List.of(new QRecord().withValue("id", id).withValue("title", "Renamed " + id)))); + } + })).isEmpty(); + } + + + + /******************************************************************************* + ** A tagged table: one assignment lookup and one vocabulary lookup per 500 + ** parents, however many records the caller asked for, plus the probe whenever + ** the node has to ask again. + *******************************************************************************/ + @Test + void taggedTableCostsOneLookupPerPage() throws Exception + { + TagEntry tag = tags.create("document", "Boundary", null); + tags.apply("document", 1, List.of(tag.id()), List.of()); + tags.apply("document", 504, List.of(tag.id()), List.of()); + assertThat(collaborationQueries(() -> assertThat(taggedIds()).containsExactlyInAnyOrder(1, 504))) + .isEqualTo(Map.of(CollaborationTables.RECORD_TAG, 2L, CollaborationTables.TAG, 2L)); + ClusterCaches.invalidate(TaggedTables.CACHE_NAME); + assertThat(collaborationQueries(() -> assertThat(taggedIds()).containsExactlyInAnyOrder(1, 504))) + .isEqualTo(Map.of(CollaborationTables.RECORD_TAG, 3L, CollaborationTables.TAG, 2L)); + } + + + + /******************************************************************************* + ** A table's first assignment shows at once on the node that wrote it, and the + ** last removal leaves the field empty again. + *******************************************************************************/ + @Test + void firstAssignmentShowsAtOnce() throws Exception + { + listWithoutTags(); + TagEntry tag = tags.create("document", "Urgent", null); + tags.apply("document", 2, List.of(tag.id()), List.of()); + assertThat(taggedIds()).containsExactly(2); + assertThat(new GetAction().executeForRecord(new GetInput("document").withPrimaryKey(2)).getValueString("collaborationTags")).isEqualTo("Urgent"); + tags.apply("document", 2, List.of(), List.of(tag.id())); + listWithoutTags(); + } + + + + /******************************************************************************* + ** The answer is forgotten when the assignment is written, so its transaction + ** reads its own tag (the service already asked again, so the list does not), + ** and again when it commits, so a probe that ran in between and could not see + ** the assignment yet does not outlive the commit. + *******************************************************************************/ + @Test + void commitForgetsAgain() throws Exception + { + listWithoutTags(); + TagEntry tag = tags.create("document", "Urgent", null); + QBackendTransaction transaction = new QBackendTransaction(); + QContext.setTransaction(transaction); + try + { + tags.apply("document", 2, List.of(tag.id()), List.of()); + assertThat(assignmentQueries(() -> assertThat(taggedIds()).containsExactly(2))).isEqualTo(2L); + transaction.commit(); + } + finally + { + QContext.clearTransaction(); + } + assertThat(assignmentQueries(() -> assertThat(taggedIds()).containsExactly(2))).isEqualTo(3L); + } + + + + /******************************************************************************* + ** An assignment this node did not write (another node, or SQL outside QQQ) + ** shows when the ClusterBus says so. + *******************************************************************************/ + @Test + void anotherNodesAssignmentShowsAfterClusterInvalidation() throws Exception + { + listWithoutTags(); + assignOutsideThisNode(tags.create("document", "Elsewhere", null)); + listWithoutTags(); + ClusterCaches.invalidateKey(TaggedTables.CACHE_NAME, "document"); + assertThat(taggedIds()).containsExactly(1); + + ClusterCaches.invalidate(TaggedTables.CACHE_NAME); + assertThat(assignmentQueries(() -> assertThat(taggedIds()).containsExactly(1))).isEqualTo(3L); + } + + + + /******************************************************************************* + ** Without a cluster transport the timeout is the bound, and 0 turns the + ** memory off: every read looks, as before. + *******************************************************************************/ + @Test + void timeoutBoundsWhatNoEventReachesAndZeroDisables() throws Exception + { + config.withTagLookupCacheSeconds(1); + listWithoutTags(); + assignOutsideThisNode(tags.create("document", "Elsewhere", null)); + listWithoutTags(); + Thread.sleep(1100); + assertThat(taggedIds()).containsExactly(1); + + config.withTagLookupCacheSeconds(0); + assertThat(assignmentQueries(() -> assertThat(taggedIds()).containsExactly(1))).isEqualTo(2L); + + List errors = new ArrayList<>(); + config.withTagLookupCacheSeconds(-1).validate(instance, errors); + assertThat(errors).contains("tagLookupCacheSeconds must be 0 (off) or more"); + } + + + + /******************************************************************************* + ** The remembered answer is the same for every session: a caller who may not + ** see a table's only tagged record must not hide its tags from one who may. + *******************************************************************************/ + @Test + void answerDoesNotDependOnTheCallersRowAccess() throws Exception + { + config.withMirroredSecurityKeyTypes(List.of("tenant")); + RecordReferenceSecurityConfig.of(instance).withMirroredSecurityKeyTypes(List.of("tenant")); + for(var table : instance.getTables().values()) + { + CoreCollaboration.reference(table, config); + } + QContext.getQSession().withSecurityKeyValue("allTenants", true); + TagEntry tag = tags.create("document", "Secret", null); + tags.apply("document", 3, List.of(tag.id()), List.of()); + assertThat(CollaborationRecords.cleanupQuery(CollaborationTables.RECORD_TAG, new QQueryFilter()).getFirst().getValueString("lockKey_tenant")).isEqualTo("b"); + + login("alice"); + ClusterCaches.invalidate(TaggedTables.CACHE_NAME); + assertThat(assignmentQueries(this::listWithoutTags)).isEqualTo(3L); + QContext.getQSession().getSecurityKeyValues().put("tenant", new ArrayList<>(List.of("b"))); + assertThat(new QueryAction().execute(new QueryInput("document")).getRecords()).extracting(row -> row.getValueString("collaborationTags")).containsExactly("Secret"); + } + + + + /******************************************************************************* + ** A static cache that is not registered would go stale on the other nodes. + *******************************************************************************/ + @Test + void cachesAreRegisteredForClusterInvalidation() throws Exception + { + listWithoutTags(); + assertThat(ClusterCacheAudit.unregisteredStaticCaches("com.kingsrook.qbits.collaboration.tags")).isEmpty(); + } + + + + /******************************************************************************* + ** Every visible parent, none of them tagged. + *******************************************************************************/ + private void listWithoutTags() throws Exception + { + assertThat(new QueryAction().execute(new QueryInput("document")).getRecords()).hasSize(PARENTS + 2) + .allSatisfy(row -> assertThat(row.getValue("collaborationTags")).isNull()); + } + + + + /******************************************************************************* + ** The ids of the visible parents that carry a tag label. + *******************************************************************************/ + private List taggedIds() throws Exception + { + return (new QueryAction().execute(new QueryInput("document")).getRecords().stream() + .filter(row -> row.getValue("collaborationTags") != null).map(row -> row.getValueInteger("id")).toList()); + } + + + + /******************************************************************************* + ** Store an assignment for document 1 without any action or listener running. + *******************************************************************************/ + private void assignOutsideThisNode(TagEntry tag) throws Exception + { + MemoryRecordStore.getInstance().insert(new InsertInput(CollaborationTables.RECORD_TAG).withRecords(List.of(new QRecord() + .withValue("tableName", "document").withValue("recordId", "1").withValue("tagId", tag.id()) + .withValue("createdByUserId", "alice").withValue("parentDeleted", false))), true); + } + + + + /******************************************************************************* + ** Backend queries against the assignment table while the work runs. + *******************************************************************************/ + private long assignmentQueries(UnsafeVoidVoidMethod work) throws Exception + { + return (collaborationQueries(work).getOrDefault(CollaborationTables.RECORD_TAG, 0L)); + } + + + + /******************************************************************************* + ** Backend queries against this qbit's tables while the work runs, by table. + *******************************************************************************/ + private Map collaborationQueries(UnsafeVoidVoidMethod work) throws Exception + { + MemoryRecordStore.resetStatistics(); + MemoryRecordStore.setCollectStatistics(true); + try + { + work.run(); + Map result = new TreeMap<>(); + List inputs = MemoryRecordStore.getActionInputs().get(QueryInput.class); + for(AbstractActionInput input : inputs == null ? List.of() : inputs) + { + String table = ((QueryInput) input).getTableName(); + if(table.startsWith(CollaborationQBitConfig.TABLE_NAME_PREFIX)) + { + result.merge(table, 1L, Long::sum); + } + } + return (result); + } + finally + { + MemoryRecordStore.setCollectStatistics(false); + MemoryRecordStore.resetStatistics(); + } + } +} -- 2.54.0 From 04d04adc6ed48e1034b1101b57c6f7c1e960f33b Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 15:24:58 -0600 Subject: [PATCH 2/2] perf(security): authorize each distinct parent once per child result page ParentRowCustomizer and AttachmentPreviewCustomizer read the parent of every child row to decide whether the row is visible, so rows that share a parent repeated the same read: 61 tasks on two parents ran 61 parent reads (TagLookupStatementBudgetTest), now 2. The answer is kept only for the one postQuery call, where the session and transaction cannot change. Also correct the README: the Tags field is selectable. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 1 + README.md | 2 +- .../AttachmentPreviewCustomizer.java | 3 +- .../security/ParentRecordAccess.java | 36 +++++++++++++++++ .../security/ParentRowCustomizer.java | 3 +- .../TagLookupStatementBudgetTest.java | 40 ++++++++++++++++--- 6 files changed, 77 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9a9a8..d171d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ All notable changes are documented here, following [Keep a Changelog](https://ke ### Fixed - Reads of a table that has no tag assignment no longer query `collaboration_record_tag`. Core selects the Tags field for every query or get that names no fields, so each of them (including per-record gets and the stored-record read before an update) ran one assignment statement whether or not the table had ever been tagged: 600 gets of an untagged table ran 600 statements, now none. Each node remembers which tables have no assignment (`TaggedTables`, core `Memoization` registered with `ClusterCaches`) for `CollaborationQBitConfig.withTagLookupCacheSeconds` (default 30, 0 disables); writing an assignment clears it on the node and through the ClusterBus. Tables with assignments are read as before, one lookup per 500 parents. No API or schema change. See "What the Tags field costs a read" in the README. +- Browsing one of the qbit's child tables (comments, tasks, record tags, attachments and the other parent-referencing tables) reads each distinct parent once per result page to authorize its rows, instead of once per row: 61 tasks on two parents ran 61 parent reads, now 2. The check itself and its outcome are unchanged. - The retained upload journal's PostgreSQL 17 qualification accepts Amazon Aurora PostgreSQL, which reports `full_page_writes=off` and does not let a deployment change it. `off` is accepted only when the settings query detects Aurora (`to_regprocedure('aurora_version()') IS NOT NULL`); stock PostgreSQL with `full_page_writes=off` is still refused, and every other requirement is unchanged on Aurora. See `docs/RETAINED-UPLOAD-JOURNAL.md`. - Bounded STRING custom fields carry QQQ's native `ValueTooLongBehavior.ERROR`, so a write longer than `maxLength` is rejected with a field error and the stored value is preserved, instead of passing through unchecked (the QQQ default `PASS_THROUGH`). - Persisted help content is reloaded after each committed runtime metadata refresh, so sections and fields created by publishing a custom-field definition receive their stored `helpContent` rows without a restart. diff --git a/README.md b/README.md index b39767f..ba46795 100644 --- a/README.md +++ b/README.md @@ -409,7 +409,7 @@ size, pending suggestions, and the five most-used approved tags. `collaborationTags` is a virtual STRING field labeled Tags with possible-value source `collaboration_tag`. `TagCriteriaExpander` supports IN, NOT_IN, EQUALS, NOT_EQUALS, IS_BLANK, IS_NOT_BLANK and CONTAINS_ALL using core `QSubQuery` membership filters. Ordinary quick search matches labels; `#name` matches normalized names. Numeric criteria are tag IDs; string criteria are names. The old ID materializer is removed. Core owns SQL execution or bounded cross-backend materialization; its bound applies to inner rows, not final parent matches. -The runtime provider adds this field and its search entry where TAGS is enabled. It is queryable but not selectable: the overlay has no virtual-field post-query hook and existing host customizers must be preserved. +The runtime provider adds this field and its search entry where TAGS is enabled. It is queryable and selectable (`TagValueProvider` fills it after host customizers have run). ### What the Tags field costs a read diff --git a/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java b/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java index bee456c..2b607ad 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java @@ -55,6 +55,7 @@ public class AttachmentPreviewCustomizer implements TableCustomizerInterface List filter(List records, boolean isGet, boolean generateUrls, AttachmentsService service) throws QException { List visible = new ArrayList<>(); + ParentRecordAccess.ResultSetParents parents = new ParentRecordAccess.ResultSetParents(); for(QRecord record : records) { try @@ -63,7 +64,7 @@ public class AttachmentPreviewCustomizer implements TableCustomizerInterface { throw (new QNotFoundException("Not found")); } - ParentRecordAccess.requireVisibleParent(record.getValueString("tableName"), record.getValueString("recordId")); + parents.requireVisible(record.getValueString("tableName"), record.getValueString("recordId")); } catch(QNotFoundException expected) { diff --git a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java index 91546a7..35703f5 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java @@ -81,6 +81,42 @@ public final class ParentRecordAccess } } + /******************************************************************************* + ** One result set's parent checks: rows that share a parent cost one lookup. + ** The session and transaction do not change within the call that creates it, + ** so never keep an instance beyond that call. + *******************************************************************************/ + public static final class ResultSetParents + { + private final java.util.Map, Boolean> visible = new java.util.HashMap<>(); + + /******************************************************************************* + ** Same outcome as ParentRecordAccess.requireVisibleParent for every row. + *******************************************************************************/ + public void requireVisible(String tableName, String recordId) throws QException + { + java.util.List parent = java.util.Arrays.asList(tableName, recordId); + Boolean known = visible.get(parent); + if(known == null) + { + try + { + requireVisibleParent(tableName, recordId); + known = true; + } + catch(QNotFoundException expected) + { + known = false; + } + visible.put(parent, known); + } + if(!known) + { + throw (new QNotFoundException("Not found")); + } + } + } + /******************************************************************************* ** Core record capability checks include parent WRITE locks for implied grants. *******************************************************************************/ diff --git a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java index b453dec..7e3ffca 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java @@ -43,6 +43,7 @@ public class ParentRowCustomizer implements TableCustomizerInterface public List postQuery(QueryOrGetInputInterface input, List records) throws QException { List visible = new ArrayList<>(); + ParentRecordAccess.ResultSetParents parents = new ParentRecordAccess.ResultSetParents(); for(QRecord record : records) { try @@ -51,7 +52,7 @@ public class ParentRowCustomizer implements TableCustomizerInterface { throw (new QNotFoundException("Not found")); } - ParentRecordAccess.requireVisibleParent(record.getValueString("tableName"), record.getValueString("recordId")); + parents.requireVisible(record.getValueString("tableName"), record.getValueString("recordId")); QRecord row = new QRecord(record); if(row.getValue("deletedAt") != null) { diff --git a/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java b/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java index 5819d4d..f44bfce 100644 --- a/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java +++ b/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; import com.kingsrook.qbits.collaboration.model.CollaborationTables; +import com.kingsrook.qbits.collaboration.permissions.CollaborationPermissions; import com.kingsrook.qbits.collaboration.tags.TagEntry; import com.kingsrook.qbits.collaboration.tags.TaggedTables; import com.kingsrook.qbits.collaboration.tags.TagsService; @@ -251,6 +252,27 @@ class TagLookupStatementBudgetTest extends PersonalServiceFixture + /******************************************************************************* + ** Browsing one of this qbit's child tables authorizes each distinct parent + ** once, not once per row (61 rows on two parents ran 61 parent reads before). + *******************************************************************************/ + @Test + void childRowsSharingAParentCostOneParentLookup() throws Exception + { + List tasks = new ArrayList<>(); + for(int id = 1; id <= 61; id++) + { + tasks.add(new QRecord().withValue("id", id).withValue("tableName", "document").withValue("recordId", id == 61 ? "2" : "1") + .withValue("title", "Task " + id).withValue("assigneeUserId", "alice").withValue("createdByUserId", "alice") + .withValue("status", "OPEN").withValue("parentDeleted", false)); + } + MemoryRecordStore.getInstance().insert(new InsertInput(CollaborationTables.TASK).withRecords(tasks), true); + login("alice", CollaborationPermissions.VIEW); + assertThat(backendQueries(() -> assertThat(new QueryAction().execute(new QueryInput(CollaborationTables.TASK)).getRecords()).hasSize(61)).get("document")).isEqualTo(2L); + } + + + /******************************************************************************* ** A static cache that is not registered would go stale on the other nodes. *******************************************************************************/ @@ -311,6 +333,18 @@ class TagLookupStatementBudgetTest extends PersonalServiceFixture ** Backend queries against this qbit's tables while the work runs, by table. *******************************************************************************/ private Map collaborationQueries(UnsafeVoidVoidMethod work) throws Exception + { + Map result = backendQueries(work); + result.keySet().removeIf(table -> !table.startsWith(CollaborationQBitConfig.TABLE_NAME_PREFIX)); + return (result); + } + + + + /******************************************************************************* + ** Every backend query while the work runs, by table. + *******************************************************************************/ + private Map backendQueries(UnsafeVoidVoidMethod work) throws Exception { MemoryRecordStore.resetStatistics(); MemoryRecordStore.setCollectStatistics(true); @@ -321,11 +355,7 @@ class TagLookupStatementBudgetTest extends PersonalServiceFixture List inputs = MemoryRecordStore.getActionInputs().get(QueryInput.class); for(AbstractActionInput input : inputs == null ? List.of() : inputs) { - String table = ((QueryInput) input).getTableName(); - if(table.startsWith(CollaborationQBitConfig.TABLE_NAME_PREFIX)) - { - result.merge(table, 1L, Long::sum); - } + result.merge(((QueryInput) input).getTableName(), 1L, Long::sum); } return (result); } -- 2.54.0