Validate index setting parameters when creating index #420
Conversation
| @@ -78,7 +78,6 @@ def add_customer_field_properties(config: Config, index_name: str, | |||
| Returns: | |||
| HTTP Response | |||
| """ | |||
| engine = "lucene" | |||
| "examples": [ | ||
| "hnsw" | ||
| ] | ||
| }, | ||
| NsFields.ann_engine: { |
There was a problem hiding this comment.
We can now let this be with the remaining parameters, but since we limit it to Lucene, it is now safe.
| @@ -117,12 +126,14 @@ | |||
| "properties": { | |||
| NsFields.hnsw_ef_construction: { | |||
| "type": "integer", | |||
| "minimum": 1, | |||
There was a problem hiding this comment.
do we need to put maximums in here? otherwise wouldn't they get a 500 from OpenSearch, if they chose a ridiculous number?
There was a problem hiding this comment.
ef_construction of 500 would actually be rather reasonable. In fact, the default is 512. We'd have to figure out what upper bounds we'd want to set. I don't think this would be self-evident.
| "examples": [ | ||
| 128 | ||
| ] | ||
| }, | ||
| NsFields.hnsw_m: { | ||
| "type": "integer", | ||
| "minimum": 1, |
There was a problem hiding this comment.
do we need to put maximums in here? otherwise wouldn't they get a 500 from OpenSearch, if they chose a ridiculous number?
There was a problem hiding this comment.
Do we have a test to ensure that allowed hnsw params can work OK?
There was a problem hiding this comment.
Do we check the index settings from OpenSearch itself, to ensure that that the kNN index has the expected settings?
There was a problem hiding this comment.
This, and the above, have been tested manually. I don't know if we want these as unit tests, not as performance or just manual testing
Changes
"ann_parameters"index settings at creation time.add_documents), that an error occurs.create_index