From 3eeaf83f3dc3c32ee944daf3f7ff9b456e88b29e Mon Sep 17 00:00:00 2001 From: luoluoyuyu Date: Fri, 23 Jan 2026 10:34:52 +0800 Subject: [PATCH 1/4] Optimize TsTable with Copy-on-Write pattern for thread-safe concurrent access - Change columnSchemaMap, tagColumnIndexMap, and idColumnIndexMap from final to volatile - Implement Copy-on-Write in executeWrite method: 1. Create local copies of maps before modification 2. Execute write operations on local copies 3. Atomically update class fields after write completes - Add WriteOperation functional interface to pass map copies to write operations - Add executeWriteWithTransform for efficient single-pass transformation during copy - Optimize renameColumnSchema to preserve column order with single-pass copy - This ensures readers see either complete old data or complete new data, avoiding ConcurrentModificationException --- .../iotdb/commons/schema/table/TsTable.java | 130 +++++++++++++----- 1 file changed, 98 insertions(+), 32 deletions(-) diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java index 1b5a9d4ac34a..f36fc0ee2880 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java @@ -73,9 +73,10 @@ public class TsTable { "When there are object fields, the %s %s shall not be '.', '..' or contain './', '.\\'."; protected String tableName; - private final Map columnSchemaMap = new LinkedHashMap<>(); - private final Map tagColumnIndexMap = new HashMap<>(); - private final Map idColumnIndexMap = new HashMap<>(); + // Copy-on-Write maps for thread-safe access without read locks + private volatile Map columnSchemaMap = new LinkedHashMap<>(); + private volatile Map tagColumnIndexMap = new HashMap<>(); + private volatile Map idColumnIndexMap = new HashMap<>(); private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); @@ -149,16 +150,27 @@ public TsTableColumnSchema getColumnSchema(final String columnName) { } /** - * Execute a write operation with optimistic lock support. This method handles the write flag and - * version increment automatically. + * Execute a write operation with Copy-on-Write support. This method creates new copies of the + * maps before modification to ensure thread-safe reads without locks. * - * @param writeOperation the write operation to execute + * @param writeOperation the write operation to execute on the new map copies */ - private void executeWrite(Runnable writeOperation) { + private void executeWrite(WriteOperation writeOperation) { readWriteLock.writeLock().lock(); isNotWrite.set(false); try { - writeOperation.run(); + // Copy-on-Write: create local copies first + Map newColumnSchemaMap = new LinkedHashMap<>(columnSchemaMap); + Map newTagColumnIndexMap = new HashMap<>(tagColumnIndexMap); + Map newIdColumnIndexMap = new HashMap<>(idColumnIndexMap); + + // Execute write operation on local copies + writeOperation.execute(newColumnSchemaMap, newTagColumnIndexMap, newIdColumnIndexMap); + + // After write completes, atomically update the class fields + columnSchemaMap = newColumnSchemaMap; + tagColumnIndexMap = newTagColumnIndexMap; + idColumnIndexMap = newIdColumnIndexMap; } finally { instanceVersion.incrementAndGet(); isNotWrite.set(true); @@ -166,6 +178,54 @@ private void executeWrite(Runnable writeOperation) { } } + /** + * Execute a write operation with a custom columnSchemaMap transformer. This allows transforming + * the map during copy (e.g., for rename operations) in a single pass. + * + * @param columnSchemaMapTransformer transforms columnSchemaMap entries during copy + * @param writeOperation the write operation to execute on the new map copies + */ + private void executeWriteWithTransform( + ColumnSchemaMapTransformer columnSchemaMapTransformer, WriteOperation writeOperation) { + readWriteLock.writeLock().lock(); + isNotWrite.set(false); + try { + // Copy-on-Write with transformation: transform columnSchemaMap in single pass + Map newColumnSchemaMap = new LinkedHashMap<>(); + for (Map.Entry entry : columnSchemaMap.entrySet()) { + columnSchemaMapTransformer.transform(entry.getKey(), entry.getValue(), newColumnSchemaMap); + } + Map newTagColumnIndexMap = new HashMap<>(tagColumnIndexMap); + Map newIdColumnIndexMap = new HashMap<>(idColumnIndexMap); + + // Execute write operation on local copies + writeOperation.execute(newColumnSchemaMap, newTagColumnIndexMap, newIdColumnIndexMap); + + // After write completes, atomically update the class fields + columnSchemaMap = newColumnSchemaMap; + tagColumnIndexMap = newTagColumnIndexMap; + idColumnIndexMap = newIdColumnIndexMap; + } finally { + instanceVersion.incrementAndGet(); + isNotWrite.set(true); + readWriteLock.writeLock().unlock(); + } + } + + @FunctionalInterface + private interface WriteOperation { + void execute( + Map columnSchemaMap, + Map tagColumnIndexMap, + Map idColumnIndexMap); + } + + @FunctionalInterface + private interface ColumnSchemaMapTransformer { + void transform( + String key, TsTableColumnSchema value, Map targetMap); + } + public int getTagColumnOrdinal(final String columnName) { readWriteLock.readLock().lock(); try { @@ -201,16 +261,16 @@ public List getTagColumnSchemaList() { // Currently only supports device view public void renameTable(final String newName) { - executeWrite(() -> tableName = newName); + executeWrite((colMap, tagMap, idMap) -> tableName = newName); } public void addColumnSchema(final TsTableColumnSchema columnSchema) { executeWrite( - () -> { - columnSchemaMap.put(columnSchema.getColumnName(), columnSchema); + (colMap, tagMap, idMap) -> { + colMap.put(columnSchema.getColumnName(), columnSchema); if (columnSchema.getColumnCategory().equals(TsTableColumnCategory.TAG)) { tagNums++; - tagColumnIndexMap.put(columnSchema.getColumnName(), tagNums - 1); + tagMap.put(columnSchema.getColumnName(), tagNums - 1); } else if (columnSchema.getColumnCategory().equals(TsTableColumnCategory.FIELD)) { fieldNum++; } @@ -218,50 +278,56 @@ public void addColumnSchema(final TsTableColumnSchema columnSchema) { } public void renameColumnSchema(final String oldName, final String newName) { - executeWrite( - () -> { - // Ensures idempotency - if (columnSchemaMap.containsKey(oldName)) { - final TsTableColumnSchema schema = columnSchemaMap.remove(oldName); + // Transform during copy: rename column while preserving insertion order in single pass + executeWriteWithTransform( + (key, schema, targetMap) -> { + if (key.equals(oldName)) { + // Rename this column while preserving its position final Map oldProps = schema.getProps(); oldProps.computeIfAbsent(TreeViewSchema.ORIGINAL_NAME, k -> schema.getColumnName()); + + TsTableColumnSchema renamedSchema; switch (schema.getColumnCategory()) { case TAG: - columnSchemaMap.put( - newName, new TagColumnSchema(newName, schema.getDataType(), oldProps)); + renamedSchema = new TagColumnSchema(newName, schema.getDataType(), oldProps); break; case FIELD: - columnSchemaMap.put( - newName, + renamedSchema = new FieldColumnSchema( newName, schema.getDataType(), ((FieldColumnSchema) schema).getEncoding(), ((FieldColumnSchema) schema).getCompressor(), - oldProps)); + oldProps); break; case ATTRIBUTE: - columnSchemaMap.put( - newName, new AttributeColumnSchema(newName, schema.getDataType(), oldProps)); + renamedSchema = new AttributeColumnSchema(newName, schema.getDataType(), oldProps); break; case TIME: default: - // Do nothing - columnSchemaMap.put(oldName, schema); + // Do nothing for TIME column + targetMap.put(key, schema); + return; } + targetMap.put(newName, renamedSchema); + } else { + targetMap.put(key, schema); } + }, + (colMap, tagMap, idMap) -> { + // No additional operation needed, transformation already done during copy }); } public void removeColumnSchema(final String columnName) { executeWrite( - () -> { - final TsTableColumnSchema columnSchema = columnSchemaMap.get(columnName); + (colMap, tagMap, idMap) -> { + final TsTableColumnSchema columnSchema = colMap.get(columnName); if (columnSchema != null && columnSchema.getColumnCategory().equals(TsTableColumnCategory.TAG)) { throw new SchemaExecutionException("Cannot remove an tag column: " + columnName); } else if (columnSchema != null) { - columnSchemaMap.remove(columnName); + colMap.remove(columnName); if (columnSchema.getColumnCategory().equals(TsTableColumnCategory.FIELD)) { fieldNum--; } @@ -354,7 +420,7 @@ public Optional getPropValue(final String propKey) { public void addProp(final String key, final String value) { executeWrite( - () -> { + (colMap, tagMap, idMap) -> { if (props == null) { props = new HashMap<>(); } @@ -364,7 +430,7 @@ public void addProp(final String key, final String value) { public void removeProp(final String key) { executeWrite( - () -> { + (colMap, tagMap, idMap) -> { if (props == null) { return; } @@ -410,7 +476,7 @@ public static TsTable deserialize(final ByteBuffer buffer) { } public void setProps(Map props) { - executeWrite(() -> this.props = props); + executeWrite((colMap, tagMap, idMap) -> this.props = props); } public void checkTableNameAndObjectNames4Object() throws MetadataException { From 9b221b192be5ecf73fb4bd23ec2a7d3009888bfd Mon Sep 17 00:00:00 2001 From: luoluoyuyu Date: Fri, 23 Jan 2026 11:15:30 +0800 Subject: [PATCH 2/4] fix --- .../org/apache/iotdb/commons/schema/table/TsTable.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java index f36fc0ee2880..2d89eb749b5a 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java @@ -185,8 +185,7 @@ private void executeWrite(WriteOperation writeOperation) { * @param columnSchemaMapTransformer transforms columnSchemaMap entries during copy * @param writeOperation the write operation to execute on the new map copies */ - private void executeWriteWithTransform( - ColumnSchemaMapTransformer columnSchemaMapTransformer, WriteOperation writeOperation) { + private void executeWriteWithTransform(ColumnSchemaMapTransformer columnSchemaMapTransformer) { readWriteLock.writeLock().lock(); isNotWrite.set(false); try { @@ -198,9 +197,6 @@ private void executeWriteWithTransform( Map newTagColumnIndexMap = new HashMap<>(tagColumnIndexMap); Map newIdColumnIndexMap = new HashMap<>(idColumnIndexMap); - // Execute write operation on local copies - writeOperation.execute(newColumnSchemaMap, newTagColumnIndexMap, newIdColumnIndexMap); - // After write completes, atomically update the class fields columnSchemaMap = newColumnSchemaMap; tagColumnIndexMap = newTagColumnIndexMap; @@ -313,9 +309,6 @@ public void renameColumnSchema(final String oldName, final String newName) { } else { targetMap.put(key, schema); } - }, - (colMap, tagMap, idMap) -> { - // No additional operation needed, transformation already done during copy }); } From 7e798b1ba71365cc7ad8a76c5e83122ead6a3744 Mon Sep 17 00:00:00 2001 From: luoluoyuyu Date: Wed, 28 Jan 2026 10:28:45 +0800 Subject: [PATCH 3/4] fix --- .../main/java/org/apache/iotdb/commons/schema/table/TsTable.java | 1 - 1 file changed, 1 deletion(-) diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java index 2d89eb749b5a..e4e4f5301a8e 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java @@ -183,7 +183,6 @@ private void executeWrite(WriteOperation writeOperation) { * the map during copy (e.g., for rename operations) in a single pass. * * @param columnSchemaMapTransformer transforms columnSchemaMap entries during copy - * @param writeOperation the write operation to execute on the new map copies */ private void executeWriteWithTransform(ColumnSchemaMapTransformer columnSchemaMapTransformer) { readWriteLock.writeLock().lock(); From 2155558217aabe2a0dd7dc09e85c569597d38b9f Mon Sep 17 00:00:00 2001 From: luoluoyuyu Date: Thu, 29 Jan 2026 17:50:38 +0800 Subject: [PATCH 4/4] fix --- .../iotdb/commons/schema/table/TsTable.java | 141 ++++++++++-------- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java index e4e4f5301a8e..68aff91c2125 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java @@ -76,7 +76,6 @@ public class TsTable { // Copy-on-Write maps for thread-safe access without read locks private volatile Map columnSchemaMap = new LinkedHashMap<>(); private volatile Map tagColumnIndexMap = new HashMap<>(); - private volatile Map idColumnIndexMap = new HashMap<>(); private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); @@ -115,7 +114,6 @@ public TsTable(String tableName, ImmutableList columnSchema public TsTable(TsTable origin) { this.tableName = origin.tableName; origin.columnSchemaMap.forEach((col, schema) -> this.columnSchemaMap.put(col, schema.copy())); - this.idColumnIndexMap.putAll(origin.idColumnIndexMap); this.props = origin.props == null ? null : new HashMap<>(origin.props); this.ttlValue = origin.ttlValue; this.tagNums = origin.tagNums; @@ -162,15 +160,13 @@ private void executeWrite(WriteOperation writeOperation) { // Copy-on-Write: create local copies first Map newColumnSchemaMap = new LinkedHashMap<>(columnSchemaMap); Map newTagColumnIndexMap = new HashMap<>(tagColumnIndexMap); - Map newIdColumnIndexMap = new HashMap<>(idColumnIndexMap); // Execute write operation on local copies - writeOperation.execute(newColumnSchemaMap, newTagColumnIndexMap, newIdColumnIndexMap); + writeOperation.execute(newColumnSchemaMap, newTagColumnIndexMap); // After write completes, atomically update the class fields columnSchemaMap = newColumnSchemaMap; tagColumnIndexMap = newTagColumnIndexMap; - idColumnIndexMap = newIdColumnIndexMap; } finally { instanceVersion.incrementAndGet(); isNotWrite.set(true); @@ -179,27 +175,48 @@ private void executeWrite(WriteOperation writeOperation) { } /** - * Execute a write operation with a custom columnSchemaMap transformer. This allows transforming - * the map during copy (e.g., for rename operations) in a single pass. + * Execute a rename operation. columnSchemaMap is modified directly, tagColumnIndexMap is copied + * for thread safety. * - * @param columnSchemaMapTransformer transforms columnSchemaMap entries during copy + * @param oldName the old column name + * @param newName the new column name + * @param renameOperation the rename operation to create renamed schema and update tag map */ - private void executeWriteWithTransform(ColumnSchemaMapTransformer columnSchemaMapTransformer) { + private void executeRename( + final String oldName, final String newName, RenameOperation renameOperation) { readWriteLock.writeLock().lock(); isNotWrite.set(false); try { - // Copy-on-Write with transformation: transform columnSchemaMap in single pass - Map newColumnSchemaMap = new LinkedHashMap<>(); + // Copy tagColumnIndexMap for thread safety + final Map newTagColumnIndexMap = new HashMap<>(tagColumnIndexMap); + + // Get expected schema before renaming + final TsTableColumnSchema expectedSchema = columnSchemaMap.get(oldName); + if (expectedSchema == null) { + return; + } + + // Rebuild columnSchemaMap in single pass + final LinkedHashMap newColumnSchemaMap = + new LinkedHashMap<>(columnSchemaMap.size()); for (Map.Entry entry : columnSchemaMap.entrySet()) { - columnSchemaMapTransformer.transform(entry.getKey(), entry.getValue(), newColumnSchemaMap); + final String currentKey = entry.getKey(); + final TsTableColumnSchema currentSchema = entry.getValue(); + + if (currentSchema == expectedSchema) { + // This is the column to rename, create renamed schema and update tag map + final TsTableColumnSchema renamedSchema = + renameOperation.createRenamedSchema(expectedSchema, newTagColumnIndexMap); + newColumnSchemaMap.put(newName, renamedSchema); + } else { + // Not the column to rename, add as is + newColumnSchemaMap.put(currentKey, currentSchema); + } } - Map newTagColumnIndexMap = new HashMap<>(tagColumnIndexMap); - Map newIdColumnIndexMap = new HashMap<>(idColumnIndexMap); - // After write completes, atomically update the class fields + // Atomically update both maps columnSchemaMap = newColumnSchemaMap; tagColumnIndexMap = newTagColumnIndexMap; - idColumnIndexMap = newIdColumnIndexMap; } finally { instanceVersion.incrementAndGet(); isNotWrite.set(true); @@ -210,15 +227,14 @@ private void executeWriteWithTransform(ColumnSchemaMapTransformer columnSchemaMa @FunctionalInterface private interface WriteOperation { void execute( - Map columnSchemaMap, - Map tagColumnIndexMap, - Map idColumnIndexMap); + Map columnSchemaMap, Map tagColumnIndexMap); } @FunctionalInterface - private interface ColumnSchemaMapTransformer { - void transform( - String key, TsTableColumnSchema value, Map targetMap); + private interface RenameOperation { + + TsTableColumnSchema createRenamedSchema( + TsTableColumnSchema expectedSchema, Map tagMap); } public int getTagColumnOrdinal(final String columnName) { @@ -256,12 +272,12 @@ public List getTagColumnSchemaList() { // Currently only supports device view public void renameTable(final String newName) { - executeWrite((colMap, tagMap, idMap) -> tableName = newName); + executeWrite((colMap, tagMap) -> tableName = newName); } public void addColumnSchema(final TsTableColumnSchema columnSchema) { executeWrite( - (colMap, tagMap, idMap) -> { + (colMap, tagMap) -> { colMap.put(columnSchema.getColumnName(), columnSchema); if (columnSchema.getColumnCategory().equals(TsTableColumnCategory.TAG)) { tagNums++; @@ -273,47 +289,48 @@ public void addColumnSchema(final TsTableColumnSchema columnSchema) { } public void renameColumnSchema(final String oldName, final String newName) { - // Transform during copy: rename column while preserving insertion order in single pass - executeWriteWithTransform( - (key, schema, targetMap) -> { - if (key.equals(oldName)) { - // Rename this column while preserving its position - final Map oldProps = schema.getProps(); - oldProps.computeIfAbsent(TreeViewSchema.ORIGINAL_NAME, k -> schema.getColumnName()); - - TsTableColumnSchema renamedSchema; - switch (schema.getColumnCategory()) { - case TAG: - renamedSchema = new TagColumnSchema(newName, schema.getDataType(), oldProps); - break; - case FIELD: - renamedSchema = - new FieldColumnSchema( - newName, - schema.getDataType(), - ((FieldColumnSchema) schema).getEncoding(), - ((FieldColumnSchema) schema).getCompressor(), - oldProps); - break; - case ATTRIBUTE: - renamedSchema = new AttributeColumnSchema(newName, schema.getDataType(), oldProps); - break; - case TIME: - default: - // Do nothing for TIME column - targetMap.put(key, schema); - return; - } - targetMap.put(newName, renamedSchema); - } else { - targetMap.put(key, schema); + executeRename( + oldName, + newName, + (expectedSchema, tagMap) -> { + // Create renamed schema + final Map props = expectedSchema.getProps(); + props.computeIfAbsent(TreeViewSchema.ORIGINAL_NAME, k -> expectedSchema.getColumnName()); + + final TsTableColumnSchema renamedSchema; + switch (expectedSchema.getColumnCategory()) { + case TAG: + renamedSchema = new TagColumnSchema(newName, expectedSchema.getDataType(), props); + // Update tagColumnIndexMap: new name's tag value is old name's value + final Integer index = tagMap.remove(oldName); + if (index != null) { + tagMap.put(newName, index); + } + break; + case FIELD: + renamedSchema = + new FieldColumnSchema( + newName, + expectedSchema.getDataType(), + ((FieldColumnSchema) expectedSchema).getEncoding(), + ((FieldColumnSchema) expectedSchema).getCompressor(), + props); + break; + case ATTRIBUTE: + renamedSchema = + new AttributeColumnSchema(newName, expectedSchema.getDataType(), props); + break; + case TIME: + default: + throw new SchemaExecutionException("TIME column cannot be renamed"); } + return renamedSchema; }); } public void removeColumnSchema(final String columnName) { executeWrite( - (colMap, tagMap, idMap) -> { + (colMap, tagMap) -> { final TsTableColumnSchema columnSchema = colMap.get(columnName); if (columnSchema != null && columnSchema.getColumnCategory().equals(TsTableColumnCategory.TAG)) { @@ -412,7 +429,7 @@ public Optional getPropValue(final String propKey) { public void addProp(final String key, final String value) { executeWrite( - (colMap, tagMap, idMap) -> { + (colMap, tagMap) -> { if (props == null) { props = new HashMap<>(); } @@ -422,7 +439,7 @@ public void addProp(final String key, final String value) { public void removeProp(final String key) { executeWrite( - (colMap, tagMap, idMap) -> { + (colMap, tagMap) -> { if (props == null) { return; } @@ -468,7 +485,7 @@ public static TsTable deserialize(final ByteBuffer buffer) { } public void setProps(Map props) { - executeWrite((colMap, tagMap, idMap) -> this.props = props); + executeWrite((colMap, tagMap) -> this.props = props); } public void checkTableNameAndObjectNames4Object() throws MetadataException {