JackieTien97 commented on code in PR #17591:
URL: https://github.com/apache/iotdb/pull/17591#discussion_r3710931845
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/utils/ResourceByPathUtils.java:
##########
@@ -221,39 +240,68 @@ protected Map<TVList, Integer> prepareTvListMapForQuery(
*/
LOGGER.debug(
"Working MemTable - clone mutable TVList and replace old TVList
in working MemTable");
- QueryContext firstQuery =
list.getQueryContextSet().iterator().next();
- // reserve query memory
- if (firstQuery instanceof FragmentInstanceContext) {
- MemoryReservationManager memoryReservationManager =
- ((FragmentInstanceContext)
firstQuery).getMemoryReservationContext();
-
memoryReservationManager.reserveMemoryCumulatively(listRamInfo.getRamSize());
- list.setReservedMemoryBytes(listRamInfo.getRamSize());
- }
- list.setOwnerQuery(firstQuery);
- // clone TVList
- cloneList = list.clone();
- cloneList.getQueryContextSet().add(context);
- tvListQueryMap.put(cloneList, cloneList.rowCount());
+ // Synchronize on memChunk to prevent concurrent clone-and-replace
races.
+ // Another query entering this same branch could call
memChunk.setWorkingTVList(cloneList)
+ // between our check above and the clone below. By locking on
memChunk we ensure only one
+ // query performs the clone-and-swap at a time.
+ synchronized (memChunk) {
+ // We re-fetch via getWorkingTVList() inside the lock because the
working TVList may
+ // have already been replaced by a concurrent query that acquired
the lock first —
+ // operating on the stale outer 'list' would clone an
already-detached copy.
+ TVList workingTVList = memChunk.getWorkingTVList();
Review Comment:
the workingTVList may not be the tvlist you lockQueryList outside.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/utils/ResourceByPathUtils.java:
##########
@@ -112,16 +116,18 @@ public abstract List<IChunkMetadata>
getVisibleMetadataListFromWriter(
* the data region write lock. At this moment, query thread holds the data
region read lock.
*
* @param context query context
- * @param memChunk writable memchunk
+ * @param memChunkRef writable memchunk reference
* @param isWorkMemTable in working or flushing memtable
* @param globalTimeFilter global time filter
* @return Map<TVList, Integer>
*/
protected Map<TVList, Integer> prepareTvListMapForQuery(
QueryContext context,
- IWritableMemChunk memChunk,
+ NodeRef<IWritableMemChunk> memChunkRef,
Review Comment:
why here need to be NodeRef?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]