Es qat support only by ZJie1 · Pull Request #10 · Intel-bigdata/IntelQATCodec

ZJie1 · 2020-02-28T09:37:23Z

No description provided.

…ed by Xie,Qi)

…y XuCheng)

…d by XuCheng)

winningsix

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.

winningsix · 2020-03-14T08:13:46Z

.gitignore

 javah/
 jni-header/
 target/
+es_qat_wrapper_backup


Remove these two.

winningsix · 2020-03-14T08:17:48Z

es_qat_wrapper/src/main/java/com/intel/qat/es/QatCompressionInputStream.java

+                .getBufferAllocatorFactory().getBufferAllocator(compressedSize);
+        this.uncompressedBuffer = uncompressedBufferAllocator
+                .allocateDirectByteBuffer(useNativeBuffer, uncompressedSize, 64);
+        this.compressedBuffer = compressedBufferAllocator


can we postpone the buffer allocate when use?

es_qat_wrapper/src/main/java/com/intel/qat/es/QatCompressionInputStream.java

winningsix · 2020-03-14T08:32:17Z

es_qat_wrapper/src/main/java/com/intel/qat/util/NativeCodeLoader.java

+import java.io.IOException;
+import java.io.InputStream;
+
+public class NativeCodeLoader {


Let's refactor the code: 1) move NativeLoader into commons module (we need to create it) 2) reuse another nativeLoader in

IntelQATCodec/spark_qat_wrapper/src/main/java/com/intel/qat/util/NativeCodeLoader.java

Line 28 in 91c0dc0

public enum NativeCodeLoader {

winningsix · 2020-03-29T04:42:53Z

es_qat_wrapper/8.0.0/elasticsearch/gradle.properties

+
+options.forkOptions.memoryMaximumSize=2g
+
+systemProp.http.proxyHost=child-prc.intel.com


Remove those please. It's companies' info. Does not apply for others.

winningsix · 2020-03-29T04:46:29Z

es_qat_wrapper/8.0.0/README.md

+    $ 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 


Rename path/to/IntelQatCodec to $IntelQATCodecSrcDri

winningsix · 2020-03-29T04:47:26Z

es_qat_wrapper/8.0.0/README.md

@@ -0,0 +1,134 @@
+#How to deploy the Elasticsearch with QAT


We can keep it in the doc. But I would suggest to change it to bash script.

winningsix · 2020-03-29T04:52:35Z

es_qat_wrapper/8.0.0/apply_es_jars.sh

+# */
+#!/bin/bash
+
+declare -a supported_Elasticsearch_versions=("8.0.0")


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

winningsix · 2020-03-29T04:53:24Z

es_qat_wrapper/8.0.0/apply_es_jars.sh

+declare -A es_lucene_version_m=(["8.0.0"]="8.2.0")
+
+# Repo Address
+ES_REPO=https://github.com/Intel-bigdata/elasticsearch.git


Can we use directly source code from ES official website?

winningsix · 2020-03-29T05:03:16Z

...0/lucene/core/src/test/org/apache/lucene/codecs/compressing/AbstractTestCompressionMode.java

+  }
+
+  public void testLUCENE5201() throws IOException {
+    byte[] data = new byte[]{


How did we get those data? Can we runtime generate them?

winningsix · 2020-03-29T05:09:26Z

...0/lucene/core/src/test/org/apache/lucene/codecs/compressing/AbstractTestCompressionMode.java

+
+  public void testDecompress() throws IOException {
+    final int iterations = atLeast(10);
+    for (int i = 0; i < iterations; ++i) {


What's the main purpose to repeat this for 10 times?

I think this is for different segments. And I will remove the changes in this class because it was just for debugging convenience.

winningsix · 2020-03-29T05:10:07Z

...0/lucene/core/src/test/org/apache/lucene/codecs/compressing/AbstractTestCompressionMode.java

+    return arr;
+  }
+
+  byte[] compress(byte[] decompressed, int off, int len) throws IOException {


can we use before/after test annotation here?

winningsix · 2020-03-29T05:11:26Z

.../core/src/test/org/apache/lucene/codecs/compressing/TestQatCompressionDecompressionMode.java

+
+package org.apache.lucene.codecs.compressing;
+
+public class TestQatCompressionDecompressionMode extends AbstractTestCompressionMode {


Empty test cases? We should move all test cases out of abstract class.

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.

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

pom.xml

winningsix · 2020-04-09T06:49:53Z

@ZJie1 Do you think this maintain mode is heavy to us?

ZJie1 added 17 commits August 30, 2019 19:13

add common modules, need changes later

f0769a6

add common module, the pom.xml in top update

d3cc105

add es_qat_wrapper modules

9d84deb

debug the es_qat_wrapper with log

996798b

[POAE7-10] ES QAT compression support test finished (Zhangjie, Review…

ac62155

…ed by Xie,Qi)

sync the function API with DeflaterCompressor

5492c0f

remove the LOG to successfully used in ES

d3f11b2

add org.apache.logging.log4j to LOG

c4ad734

clean code

24552f1

[POAE7-10]ES QAT compression clean code finished(Zhangjie, Reviewed b…

67d5a5e

…y XuCheng)

[POAE7-10]ES QAT compression clean code finished(Zhangjie, Reviewed b…

cc2d8bd

…y XuCheng)

add LOG in read and write

3a4d88b

[POAE7-10]Add Log in Write and Read to identify QAT(Zhangjie, Reviewe…

282e7b2

…d by XuCheng)

change log level

bacaeb1

[POAE7-10](add Hive docs,Zhangjie, Reviewed by XuCheng)

12d0f87

[POAE7-10](add Hive docs,Zhangjie, Reviewed by XuCheng)

a35f856

add patch of es that with qat and changed lucene

7852409

winningsix suggested changes Feb 28, 2020

View reviewed changes

ZJie1 added 2 commits March 3, 2020 10:38

remove unnecessary changes

6ddf108

delete unused log dependency in pom

85996a9

winningsix suggested changes Mar 14, 2020

View reviewed changes

add the apply patches for elasticsearch and lucene with version info

14e02b6

winningsix suggested changes Mar 29, 2020

View reviewed changes

add patches of elasticsearch version 7.6.1

0c64080

winningsix reviewed Mar 31, 2020

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

ZJie1 added 2 commits April 3, 2020 22:57

using diff files to apply patches for v7.6.1

c457064

delete the version 8.0.0

87e4e02

winningsix closed this Apr 9, 2020

winningsix reopened this Apr 9, 2020

set compression level to 1

07c813c


		options.forkOptions.memoryMaximumSize=2g

		systemProp.http.proxyHost=child-prc.intel.com


		package org.apache.lucene.codecs.compressing;

		public class TestQatCompressionDecompressionMode extends AbstractTestCompressionMode {

Conversation

ZJie1 commented Feb 28, 2020

Uh oh!

winningsix left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

winningsix commented Apr 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants