Skip to content

Commit 4e101a0

Browse files
yhmoyhmo
committed
Fix a bug that index property was not correctly passed
Signed-off-by: yhmo <yihua.mo@zilliz.com>
1 parent 65f6405 commit 4e101a0

8 files changed

Lines changed: 79 additions & 25 deletions

File tree

‎sdk-core/src/main/java/io/milvus/client/AbstractMilvusGrpcClient.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.google.common.collect.Lists;
2323
import com.google.common.util.concurrent.*;
24+
import com.google.gson.reflect.TypeToken;
2425
import io.grpc.StatusRuntimeException;
2526
import io.milvus.common.utils.GTsDict;
2627
import io.milvus.common.utils.JsonUtils;
@@ -1331,9 +1332,22 @@ public R<RpcStatus> createIndex(@NonNull CreateIndexParam requestParam) {
13311332
try {
13321333
// prepare index parameters
13331334
CreateIndexRequest.Builder createIndexRequestBuilder = CreateIndexRequest.newBuilder();
1334-
List<KeyValuePair> extraParamList = ParamUtils.AssembleKvPair(requestParam.getExtraParam());
1335-
if (CollectionUtils.isNotEmpty(extraParamList)) {
1336-
extraParamList.forEach(createIndexRequestBuilder::addExtraParams);
1335+
Map<String, String> extraParams = requestParam.getExtraParam();
1336+
for (Map.Entry<String, String> entry : extraParams.entrySet()) {
1337+
if (entry.getKey().equals(Constant.PARAMS)) {
1338+
Map<String, String> tempParams = JsonUtils.fromJson(entry.getValue(), new TypeToken<Map<String, String>>() {}.getType());
1339+
for (String key : tempParams.keySet()) {
1340+
createIndexRequestBuilder.addExtraParams(KeyValuePair.newBuilder()
1341+
.setKey(key)
1342+
.setValue(tempParams.get(key))
1343+
.build());
1344+
}
1345+
} else {
1346+
createIndexRequestBuilder.addExtraParams(KeyValuePair.newBuilder()
1347+
.setKey(entry.getKey())
1348+
.setValue(entry.getValue())
1349+
.build());
1350+
}
13371351
}
13381352

13391353
CreateIndexRequest.Builder builder = createIndexRequestBuilder

‎sdk-core/src/main/java/io/milvus/response/DescIndexResponseWrapper.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package io.milvus.response;
2121

22+
import com.google.gson.reflect.TypeToken;
23+
import io.milvus.common.utils.JsonUtils;
2224
import io.milvus.grpc.IndexDescription;
2325
import io.milvus.grpc.DescribeIndexResponse;
2426

@@ -149,12 +151,18 @@ public MetricType getMetricType() {
149151
}
150152

151153
public String getExtraParam() {
152-
if (this.params.containsKey(Constant.PARAMS)) {
153-
// may throw IllegalArgumentException
154-
return params.get(Constant.PARAMS);
154+
Map<String, String> extraParams = new HashMap<>();
155+
for (Map.Entry<String, String> entry : this.params.entrySet()) {
156+
if (entry.getKey().equals(Constant.INDEX_TYPE) || entry.getKey().equals(Constant.METRIC_TYPE)) {
157+
} else if (entry.getKey().equals(Constant.PARAMS)) {
158+
Map<String, String> tempParams = JsonUtils.fromJson(entry.getValue(), new TypeToken<Map<String, String>>() {}.getType());
159+
extraParams.putAll(tempParams);
160+
} else {
161+
extraParams.put(entry.getKey(), entry.getValue());
162+
}
155163
}
156164

157-
return "";
165+
return JsonUtils.toJson(extraParams);
158166
}
159167
}
160168
}

‎sdk-core/src/main/java/io/milvus/v2/service/index/IndexService.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,12 @@ public Void createIndex(MilvusServiceGrpc.MilvusServiceBlockingStub blockingStub
6666
}
6767
Map<String, Object> extraParams = indexParam.getExtraParams();
6868
if (extraParams != null && !extraParams.isEmpty()) {
69-
JsonObject params = new JsonObject();
7069
for (String key : extraParams.keySet()) {
71-
params.addProperty(key, extraParams.get(key).toString());
70+
builder.addExtraParams(KeyValuePair.newBuilder()
71+
.setKey(key)
72+
.setValue(extraParams.get(key).toString())
73+
.build());
7274
}
73-
// the extra params is a JSON format string like "{\"M\": 8, \"efConstruction\": 64}"
74-
builder.addExtraParams(KeyValuePair.newBuilder()
75-
.setKey(Constant.PARAMS)
76-
.setValue(params.toString())
77-
.build());
7875
}
7976

8077
Status status = blockingStub.createIndex(builder.build());

‎sdk-core/src/main/java/io/milvus/v2/service/index/response/DescribeIndexResp.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,17 @@ public static final class IndexDesc {
7777
private IndexBuildState indexState = IndexBuildState.IndexStateNone;
7878
@Builder.Default
7979
String indexFailedReason = "";
80+
81+
// In 2.4/2.5, properties only contains one item "mmap.enabled".
82+
// To keep consistence with other SDKs, we intend to remove this member from IndexDesc,
83+
// and put "mmap.enabled" into the "extraParams", the reasons:
84+
// (1) when createIndex() is call, "mmap.enabled" is passed by the IndexParam.extraParams
85+
// (2) other SDKs don't have a "properties" member for describeIndex()
86+
// (3) now the "mmap.enabled" is dispatched to "properties" by ConvertUtils.convertToDescribeIndexResp(),
87+
// once there are new property available, the new property will be dispatched to "extraParams",
88+
// the "properties" member is not maintainable.
8089
@Builder.Default
90+
@Deprecated
8191
private Map<String, String> properties = new HashMap<>();
8292
}
8393
}

‎sdk-core/src/main/java/io/milvus/v2/utils/ConvertUtils.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,14 @@ public DescribeIndexResp convertToDescribeIndexResp(List<IndexDescription> respo
9797
// may throw IllegalArgumentException
9898
metricType = IndexParam.MetricType.valueOf(param.getValue());
9999
} else if (param.getKey().equals(Constant.MMAP_ENABLED)) {
100-
properties.put(param.getKey(), param.getValue());
100+
properties.put(param.getKey(), param.getValue()); // just for compatible with old versions
101+
extraParams.put(param.getKey(), param.getValue());
101102
} else if (param.getKey().equals(Constant.PARAMS)) {
102-
extraParams = JsonUtils.fromJson(param.getValue(), new TypeToken<Map<String, String>>() {}.getType());
103+
Map<String, String> tempParams = JsonUtils.fromJson(param.getValue(), new TypeToken<Map<String, String>>() {}.getType());
104+
tempParams.remove(Constant.MMAP_ENABLED); // "mmap.enabled" in "params" is not processed by server
105+
extraParams.putAll(tempParams);
106+
} else {
107+
extraParams.put(param.getKey(), param.getValue());
103108
}
104109
}
105110

‎sdk-core/src/test/java/io/milvus/client/MilvusClientDockerTest.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,7 @@
4141
import io.milvus.param.highlevel.dml.GetIdsParam;
4242
import io.milvus.param.highlevel.dml.response.DeleteResponse;
4343
import io.milvus.param.highlevel.dml.response.GetResponse;
44-
import io.milvus.param.index.CreateIndexParam;
45-
import io.milvus.param.index.DescribeIndexParam;
46-
import io.milvus.param.index.DropIndexParam;
47-
import io.milvus.param.index.GetIndexStateParam;
44+
import io.milvus.param.index.*;
4845
import io.milvus.param.partition.GetPartitionStatisticsParam;
4946
import io.milvus.param.partition.ShowPartitionsParam;
5047
import io.milvus.pool.MilvusClientV1Pool;
@@ -457,13 +454,14 @@ void testFloatVectors() {
457454
Assertions.assertEquals(R.Status.Success.getCode(), createIndexR.getStatus().intValue());
458455

459456
// create index on vector field
457+
String params = "{\"efConstruction\":64,\"M\":16,\"mmap.enabled\":true}";
460458
indexParam = CreateIndexParam.newBuilder()
461459
.withCollectionName(randomCollectionName)
462460
.withFieldName(DataType.FloatVector.name())
463461
.withIndexName("abv")
464462
.withIndexType(IndexType.HNSW)
465463
.withMetricType(MetricType.L2)
466-
.withExtraParam("{\"M\":16,\"efConstruction\":64}")
464+
.withExtraParam(params)
467465
.withSyncMode(Boolean.TRUE)
468466
.withSyncWaitingInterval(500L)
469467
.withSyncWaitingTimeout(30L)
@@ -491,8 +489,24 @@ void testFloatVectors() {
491489
Assertions.assertEquals(rowCount, indexDesc.getIndexedRows());
492490
Assertions.assertEquals(0L, indexDesc.getPendingIndexRows());
493491
Assertions.assertTrue(indexDesc.getIndexFailedReason().isEmpty());
492+
String extraParams = indexDesc.getExtraParam();
493+
Assertions.assertEquals(params.replace("\"", ""), extraParams.replace("\"", ""));
494494
System.out.println("Index description: " + indexDesc.toString());
495495

496+
R<RpcStatus> alterR = client.alterIndex(AlterIndexParam.newBuilder()
497+
.withCollectionName(randomCollectionName)
498+
.withIndexName("abv")
499+
.withMMapEnabled(false)
500+
.build());
501+
Assertions.assertEquals(R.Status.Success.getCode(), alterR.getStatus().intValue());
502+
503+
descIndexR = client.describeIndex(descIndexParam);
504+
Assertions.assertEquals(R.Status.Success.getCode(), descIndexR.getStatus().intValue());
505+
indexDescWrapper = new DescIndexResponseWrapper(descIndexR.getData());
506+
indexDesc = indexDescWrapper.getIndexDescByFieldName(DataType.FloatVector.name());
507+
extraParams = indexDesc.getExtraParam();
508+
Assertions.assertEquals("{efConstruction:64,M:16,mmap.enabled:false}", extraParams.replace("\"", ""));
509+
496510
// load collection
497511
R<RpcStatus> loadR = client.loadCollection(LoadCollectionParam.newBuilder()
498512
.withCollectionName(randomCollectionName)

‎sdk-core/src/test/java/io/milvus/client/MilvusServiceClientTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,7 +1430,7 @@ void createIndex() {
14301430
.withFieldName("aaa")
14311431
.withIndexType(IndexType.IVF_FLAT)
14321432
.withMetricType(MetricType.L2)
1433-
.withExtraParam("dummy")
1433+
.withExtraParam("{\"dummy\": 0}")
14341434
.withSyncMode(Boolean.TRUE)
14351435
.withSyncWaitingInterval(500L)
14361436
.withSyncWaitingTimeout(2L)
@@ -1445,7 +1445,7 @@ void createIndex() {
14451445
.withFieldName("field1")
14461446
.withIndexType(IndexType.BIN_IVF_FLAT)
14471447
.withMetricType(MetricType.L2)
1448-
.withExtraParam("dummy")
1448+
.withExtraParam("{\"dummy\": 1}")
14491449
.withSyncMode(Boolean.TRUE)
14501450
.withSyncWaitingInterval(500L)
14511451
.withSyncWaitingTimeout(2L)
@@ -1472,7 +1472,7 @@ void createIndex() {
14721472
.withFieldName("field1")
14731473
.withIndexType(IndexType.IVF_FLAT)
14741474
.withMetricType(MetricType.L2)
1475-
.withExtraParam("dummy")
1475+
.withExtraParam("{\"dummy\": 2}")
14761476
.withSyncMode(Boolean.TRUE)
14771477
.withSyncWaitingInterval(500L)
14781478
.withSyncWaitingTimeout(2L)
@@ -2737,7 +2737,8 @@ void testDescIndexResponseWrapper() {
27372737
assertEquals(fieldName, indexDesc.getFieldName());
27382738
assertEquals(indexType, indexDesc.getIndexType());
27392739
assertEquals(metricType, indexDesc.getMetricType());
2740-
assertEquals(0, extraParam.compareTo(indexDesc.getExtraParam()));
2740+
String params = indexDesc.getExtraParam();
2741+
assertEquals(0, extraParam.compareTo(params.replace("\"", "")));
27412742

27422743
assertFalse(wrapper.toString().isEmpty());
27432744
}

‎sdk-core/src/test/java/io/milvus/v2/client/MilvusClientV2DockerTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,9 @@ void testIndex() {
12481248
Map<String, String> indexProps = desc.getProperties();
12491249
Assertions.assertTrue(indexProps.containsKey(Constant.MMAP_ENABLED));
12501250
Assertions.assertEquals("false", indexProps.get(Constant.MMAP_ENABLED));
1251+
extraParams = desc.getExtraParams();
1252+
Assertions.assertTrue(extraParams.containsKey(Constant.MMAP_ENABLED));
1253+
Assertions.assertEquals("false", extraParams.get(Constant.MMAP_ENABLED));
12511254

12521255
client.dropIndexProperties(DropIndexPropertiesReq.builder()
12531256
.collectionName(randomCollectionName)
@@ -1261,6 +1264,8 @@ void testIndex() {
12611264
desc = descResp.getIndexDescByFieldName("vector");
12621265
indexProps = desc.getProperties();
12631266
Assertions.assertFalse(indexProps.containsKey(Constant.MMAP_ENABLED));
1267+
extraParams = desc.getExtraParams();
1268+
Assertions.assertFalse(extraParams.containsKey(Constant.MMAP_ENABLED));
12641269

12651270
// drop index
12661271
client.dropIndex(DropIndexReq.builder()

0 commit comments

Comments
 (0)