timoninmaxim commented on code in PR #12412:
URL: https://github.com/apache/ignite/pull/12412#discussion_r2513951411
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexProcessor.java:
##########
@@ -383,16 +425,35 @@ public IndexesRebuildTask idxRebuild() {
* @return Collection of indexes for specified cache.
*/
public Collection<Index> indexes(String cacheName) {
+ return indexes(cacheName, false);
+ }
+
+ /**
+ * Returns collection of indexes for specified cache.
+ *
+ * @param cacheName Cache name.
+ * @param skipFilling If {@code true}, indexes that are currently being
initially populated are excluded
+ * from the result; if {@code false}, all known indexes for the cache
are returned.
+ * @return Collection of indexes for specified cache.
+ */
+
+ public Collection<Index> indexes(String cacheName, boolean skipFilling) {
ddlLock.readLock().lock();
try {
- Map<String, Index> idxs = cacheToIdx.get(cacheName);
+ Map<String, Index> idxMap = cacheToIdx.get(cacheName);
- if (idxs == null)
+ if (idxMap == null)
return Collections.emptyList();
- return idxs.values();
+ Collection<Index> idxs = idxMap.values();
+
+ if (!skipFilling || fillingIdxs == null)
+ return new ArrayList<>(idxs);
Review Comment:
No need to wrap the collection to new ArrayList. This code is for internal
Ignite usage only.
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexProcessor.java:
##########
@@ -383,16 +425,35 @@ public IndexesRebuildTask idxRebuild() {
* @return Collection of indexes for specified cache.
*/
public Collection<Index> indexes(String cacheName) {
+ return indexes(cacheName, false);
+ }
+
+ /**
+ * Returns collection of indexes for specified cache.
+ *
+ * @param cacheName Cache name.
+ * @param skipFilling If {@code true}, indexes that are currently being
initially populated are excluded
+ * from the result; if {@code false}, all known indexes for the cache
are returned.
+ * @return Collection of indexes for specified cache.
+ */
+
+ public Collection<Index> indexes(String cacheName, boolean skipFilling) {
ddlLock.readLock().lock();
try {
- Map<String, Index> idxs = cacheToIdx.get(cacheName);
+ Map<String, Index> idxMap = cacheToIdx.get(cacheName);
- if (idxs == null)
+ if (idxMap == null)
return Collections.emptyList();
- return idxs.values();
+ Collection<Index> idxs = idxMap.values();
+
+ if (!skipFilling || fillingIdxs == null)
+ return new ArrayList<>(idxs);
+ return idxs.stream().filter(idx ->
Review Comment:
Streams are heavy, avoid using them
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexProcessor.java:
##########
@@ -218,15 +224,43 @@ public Index createIndexDynamically(
IndexDefinition definition,
SchemaIndexCacheVisitor cacheVisitor
) {
- Index idx = createIndex(cctx, factory, definition);
+ IndexFactory dynamicFactory = (gcctx, indexDefinition) -> {
+ Index idx = factory.createIndex(gcctx, indexDefinition);
+
+ // Under lock.
+ if (fillingIdxs == null)
+ fillingIdxs = new HashSet<>();
+ fillingIdxs.add(idx.indexDefinition().idxName());
Review Comment:
add NL before
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexProcessor.java:
##########
@@ -218,15 +224,43 @@ public Index createIndexDynamically(
IndexDefinition definition,
SchemaIndexCacheVisitor cacheVisitor
) {
- Index idx = createIndex(cctx, factory, definition);
+ IndexFactory dynamicFactory = (gcctx, indexDefinition) -> {
+ Index idx = factory.createIndex(gcctx, indexDefinition);
+
+ // Under lock.
Review Comment:
Instead of comment use `assert ddlLock.isWriteLockedByCurrentThread();`
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexProcessor.java:
##########
@@ -218,15 +224,43 @@ public Index createIndexDynamically(
IndexDefinition definition,
SchemaIndexCacheVisitor cacheVisitor
) {
- Index idx = createIndex(cctx, factory, definition);
+ IndexFactory dynamicFactory = (gcctx, indexDefinition) -> {
+ Index idx = factory.createIndex(gcctx, indexDefinition);
+
+ // Under lock.
+ if (fillingIdxs == null)
+ fillingIdxs = new HashSet<>();
+ fillingIdxs.add(idx.indexDefinition().idxName());
+
+ return idx;
+ };
+
+ Index idx = createIndex(cctx, dynamicFactory, definition);
Review Comment:
Looks like this call should be in the try-catch block also. For a visitor
lambda we can use IndexName from the definition variable.
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexQueryProcessor.java:
##########
@@ -455,4 +455,5 @@ public static String rangeDesc(RangeIndexQueryCriterion c,
String fldName, Objec
return r.toString();
}
+
Review Comment:
Useless change
--
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]
