Adds Open Distro Elastic Search's KNN plugin support. Closes #174.

[stephenleo]

Adding Open Distro Elasticsearch KNN plugin support by borrowing on the ES setup from elasticsearch and elastiknn. Below is a comparison on fashion-mnist between this opendistroknn and elastiknn, where opnedistroknn has ~3X better queries/s at comparable recall.

[stephenleo]

@alexklibisz could you pls help to review?
I have 2 questions:

1. I tried increasing the JVM memory (Xms and Xmx) together with an increase in translog.flush_threshold_size as discussed in Link but didn't seem to impact the speed here. So seems like we are CPU bottlenecked instead of memory right?
2. We cannot increase knn.algo_param.index_thread_qty since we want to limit it to only 1 core right?

[erikbern]

Yeah, you're limited in terms of memory and cores by Docker, so I don't think it will bring anything to change config within the container. Almost all algorithms benefit from more memory/CPU/cores so we try to keep the benchmark simple.

[erikbern]

This looks great – thanks!

[alexklibisz]

1. I tried increasing the JVM memory (Xms and Xmx) together with an increase in translog.flush_threshold_size as discussed in Link but didn't seem to impact the speed here. So seems like we are CPU bottlenecked instead of memory right?

Almost certainly CPU bottlenecked, but you never know without profiling. You can add ports={"8097":"8097"} as an arg to this method call, run the runner, open VisualVM, and it should recognize that there's an Elasticsearch JVM running and let you view memory usage and sample CPU usage. I was able to run Elasticsearch and Elastiknn comfortably with a 3GB heap, but opendistro is running an additional process for HNSW. I'd also suggest booting up an EC2 c5.xlarge or something with a similar memory profile and making sure you can run all the way through with --parallelism 3, since that's the setting @erikbern uses.

2. We cannot increase knn.algo_param.index_thread_qty since we want to limit it to only 1 core right?

I'm not sure about that setting, but yes you generally want to specify parallelism=1 at every level. The runner pins your container to a single core, so if you try to do parallel CPU-bound work on many threads you'll either get killed by the container runtime or waste time context switching.

[stephenleo]

Thank you for reviewing @alexklibisz . I think EC2 c5.xlarge is insufficient for --parallelism 3 since it only has 8GB RAM while we need 3*3GB. Instead, I've run it successfully on a GCP n1-standard-4 (4 vCPUs, 15 GB RAM) machine.

Here is a snapshot of docker stats during the query (I wasn't successful in getting VisualVM to show the stats). I'm not sure why the memory usage for each container is exceeding 4GB despite setting -Xmx3G. What do you think?

CONTAINER ID        NAME                CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BLOCK I/O           PIDS
21d5eae537df        beautiful_hermann   98.53%              4.019GiB / 4.511GiB   89.10%              1.44kB / 0B         23.3MB / 1.11GB     59
73b22733b190        dazzling_swartz     98.79%              4.209GiB / 4.512GiB   93.29%              1.44kB / 0B         43.5MB / 942MB      59
5d08d3625cfa        pedantic_ishizaka   98.95%              4.24GiB / 4.511GiB    93.98%              1.44kB / 0B         722MB / 571MB       59

[alexklibisz]

Oops I meant 4xlarge! Must’ve been a typo. Opendistro is using nmslib to run the hnsw model. That likely consumes a chunk of memory. Also the JVM Itself uses memory. The 3gb is just what’s allocated for ES.

…

On Sun, Dec 13, 2020 at 3:18 AM Marie Stephen Leo ***@***.***> wrote: Thank you for reviewing @alexklibisz <https://github.com/alexklibisz> . I think EC2 c5.xlarge is insufficient for --parallelism 3 since it only has 8GB RAM while we need 3*3GB. Instead, I've run it successfully on a GCP n1-standard-4 (4 vCPUs, 15 GB RAM) machine. Here is a snapshot of docker stats during the query (I wasn't successful in getting VisualVM to show the stats). I'm not sure why the memory usage for each container is exceeding 4GB despite setting -Xmx3G. What do you think? CONTAINER ID NAME CPU % MEM USAGE / LIMIT MEM % NET I/O BLOCK I/O PIDS 21d5eae537df beautiful_hermann 98.53% 4.019GiB / 4.511GiB 89.10% 1.44kB / 0B 23.3MB / 1.11GB 59 73b22733b190 dazzling_swartz 98.79% 4.209GiB / 4.512GiB 93.29% 1.44kB / 0B 43.5MB / 942MB 59 5d08d3625cfa pedantic_ishizaka 98.95% 4.24GiB / 4.511GiB 93.98% 1.44kB / 0B 722MB / 571MB 59 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#202 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5E27DG46DWTYTVI23EZ5DSUR2FBANCNFSM4UXTGSLQ> .

[erikbern]

is this ready to be merged?

[stephenleo]

I'm good to merge unless @alexklibisz has any concerns?

[alexklibisz]

LGTM. Excited to see all the results side-by-side.

[alexklibisz]

@erikbern slight tangent: did travis get removed for the repo? If you'd be interested, I've recently converted some other repos to use Github Actions. I could take a pass at that here.

[erikbern]

i'm not sure what happened to travis. it's still running, but something is broken with the PR integration, I think

will merge this!

[stephenleo]

Strange, the master build failed. Though the PR itself passed...
Pls, let me know if there is anything I should fix

[erikbern]

i'll take a look at it

[stephenleo]

found the issue @erikbern , open distro 1.12.0 was released on 14th and breaks the installation. I've submitted a PR with the fix.

[maumueller]

@erikbern slight tangent: did travis get removed for the repo? If you'd be interested, I've recently converted some other repos to use Github Actions. I could take a pass at that here.

I would actually like to see CI via github actions. I like that we could just upload the produced plot artifacts to the builds.

[alexklibisz]

I'm not sure if I follow this part:

I like that we could just upload the produced plot artifacts to the builds.

[maumueller]

Sorry for the imprecision.

I would like to see the plots generated via https://github.com/erikbern/ann-benchmarks/blob/master/.travis.yml#L43-L44 to be uploaded to the CI build via https://github.com/actions/upload-artifact. This could be useful for performance bug hunting.

[erikbern]

that would be a good idea – I'd be supportive of the change!