Es qat support only #10
Conversation
There was a problem hiding this comment.
Two major comments: 1) could you remove unnecessary code changes like Hive and avoid committing .swp file; 2) the largest file is ES diff file. The diff may be due to different code styles in your local IDE from upstream. Can you revert unnecessary changes here? We will have multi-versions support for different components. So we will do patch for different versions. We need to keep changes as little as possible to lower the burden.
.gitignore
Outdated
| @@ -39,3 +39,5 @@ columnar_format_qat_wrapper/target/ | |||
| javah/ | |||
| jni-header/ | |||
| target/ | |||
| es_qat_wrapper_backup | |||
| .getBufferAllocatorFactory().getBufferAllocator(compressedSize); | ||
| this.uncompressedBuffer = uncompressedBufferAllocator | ||
| .allocateDirectByteBuffer(useNativeBuffer, uncompressedSize, 64); | ||
| this.compressedBuffer = compressedBufferAllocator |
There was a problem hiding this comment.
can we postpone the buffer allocate when use?
es_qat_wrapper/src/main/java/com/intel/qat/es/QatCompressionInputStream.java
Show resolved
Hide resolved
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
|
|
||
| public class NativeCodeLoader { |
There was a problem hiding this comment.
Let's refactor the code: 1) move NativeLoader into commons module (we need to create it) 2) reuse another nativeLoader in
|
|
||
| options.forkOptions.memoryMaximumSize=2g | ||
|
|
||
| systemProp.http.proxyHost=child-prc.intel.com |
There was a problem hiding this comment.
Remove those please. It's companies' info. Does not apply for others.
es_qat_wrapper/8.0.0/README.md
Outdated
| $ mvn clean install -Dqatzip.libs=/opt/intel/QATzip -Dqatzip.src=/opt/intel/QATzip -DskipTests | ||
| Then we can get the following files that we need. | ||
|
|
||
| path/to/IntelQatCodec/lucene_qat_wrapper/target/lucene_qat_wrapper.jar |
There was a problem hiding this comment.
Rename path/to/IntelQatCodec to $IntelQATCodecSrcDri
es_qat_wrapper/8.0.0/README.md
Outdated
| @@ -0,0 +1,134 @@ | |||
| #How to deploy the Elasticsearch with QAT | |||
There was a problem hiding this comment.
We can keep it in the doc. But I would suggest to change it to bash script.
| # */ | ||
| #!/bin/bash | ||
|
|
||
| declare -a supported_Elasticsearch_versions=("8.0.0") |
There was a problem hiding this comment.
It seems 8.0.0 has not been released. We should use a released version. https://www.elastic.co/guide/en/elasticsearch/reference/current/es-release-notes.html
| declare -A es_lucene_version_m=(["8.0.0"]="8.2.0") | ||
|
|
||
| # Repo Address | ||
| ES_REPO=https://github.com/Intel-bigdata/elasticsearch.git |
There was a problem hiding this comment.
Can we use directly source code from ES official website?
| } | ||
|
|
||
| public void testLUCENE5201() throws IOException { | ||
| byte[] data = new byte[]{ |
There was a problem hiding this comment.
How did we get those data? Can we runtime generate them?
|
|
||
| public void testDecompress() throws IOException { | ||
| final int iterations = atLeast(10); | ||
| for (int i = 0; i < iterations; ++i) { |
There was a problem hiding this comment.
What's the main purpose to repeat this for 10 times?
There was a problem hiding this comment.
I think this is for different segments. And I will remove the changes in this class because it was just for debugging convenience.
| return arr; | ||
| } | ||
|
|
||
| byte[] compress(byte[] decompressed, int off, int len) throws IOException { |
There was a problem hiding this comment.
can we use before/after test annotation here?
|
|
||
| package org.apache.lucene.codecs.compressing; | ||
|
|
||
| public class TestQatCompressionDecompressionMode extends AbstractTestCompressionMode { |
There was a problem hiding this comment.
Empty test cases? We should move all test cases out of abstract class.
There was a problem hiding this comment.
No, it's not empty. Lucene has its own test code. Different CompressionMode will call the same code from AbstractTestCompressionMode. We just need to extend it.
There was a problem hiding this comment.
I mean you can move those test cases from abstract class to this class.
...8.0.0/elasticsearch/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
Outdated
Show resolved
Hide resolved
|
@ZJie1 Do you think this maintain mode is heavy to us? |
No description provided.