You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/09/12 12:21:10 UTC

[GitHub] [pulsar-client-node] savearray2 opened a new pull request #121: Includes Support for ZSTD and SNAPPY Compression

savearray2 opened a new pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121


   A version bump to the latest version of the Pulsar C++ library (2.6.1) was required to make use of the compression types. As the latest version of the C++ library requires ```GLIBCXX_3.4.22```, the testing suite code was altered to automatically update the Docker image with the latest version of ```libstdc++6``` and ```gcc-4.9``` before executing the library tests.
   
   Since this makes use of a later version of the C++ library, compatibility requirements will change, and should probably be reflected in ```README.md```.
   
   Accommodates the issue created here: https://github.com/apache/pulsar-client-node/issues/11#issue-420827873


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490745802



##########
File path: pulsar-version.txt
##########
@@ -1 +1 @@
-2.5.0
+2.6.1

Review comment:
       ZSTD and SNAPPY seem to be available from Pulsar v2.6.0.
   https://github.com/apache/pulsar/pull/7014




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] savearray2 commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
savearray2 commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490864912



##########
File path: pulsar-version.txt
##########
@@ -1 +1 @@
-2.5.0
+2.6.1

Review comment:
       Would you like me to change this to ```2.6.0``` instead of ```2.6.1```?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam merged pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam merged pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam merged pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam merged pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490811096



##########
File path: tests/end_to_end.test.js
##########
@@ -376,5 +376,53 @@ const Pulsar = require('../index.js');
       await reader.close();
       await client.close();
     });
+
+    test('Produce/Read (Compression)', async () => {
+      const client = new Pulsar.Client({
+        serviceUrl: 'pulsar://localhost:6650',
+        operationTimeoutSeconds: 30,
+      });
+      expect(client).not.toBeNull();
+
+      const topic = 'persistent://public/default/produce-read-compression';
+      const producer = await client.createProducer({
+        topic,
+        sendTimeoutMs: 30000,
+        batchingEnabled: true,
+        compressionType: 'ZSTD',

Review comment:
       I don't think SNAPPY is available unless the SNAPPY library is installed when the C++ client DEB package is built.
   https://github.com/apache/pulsar/pull/4259/files#diff-bda8db832e446d092d38464123ed17bc
   https://github.com/apache/pulsar/pull/4259/files#diff-7e441fcfcb20e339c71f7f8f48569579
   
   So, even if you modify the Docker image, we cannot use SNAPPY immediately. We have to wait for the next release of the C++ package.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490747820



##########
File path: tests/end_to_end.test.js
##########
@@ -376,5 +376,53 @@ const Pulsar = require('../index.js');
       await reader.close();
       await client.close();
     });
+
+    test('Produce/Read (Compression)', async () => {
+      const client = new Pulsar.Client({
+        serviceUrl: 'pulsar://localhost:6650',
+        operationTimeoutSeconds: 30,
+      });
+      expect(client).not.toBeNull();
+
+      const topic = 'persistent://public/default/produce-read-compression';
+      const producer = await client.createProducer({
+        topic,
+        sendTimeoutMs: 30000,
+        batchingEnabled: true,
+        compressionType: 'ZSTD',

Review comment:
       Could you add a test for SNAPPY as well?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490757691



##########
File path: tests/end_to_end.test.js
##########
@@ -376,5 +376,53 @@ const Pulsar = require('../index.js');
       await reader.close();
       await client.close();
     });
+
+    test('Produce/Read (Compression)', async () => {
+      const client = new Pulsar.Client({
+        serviceUrl: 'pulsar://localhost:6650',
+        operationTimeoutSeconds: 30,
+      });
+      expect(client).not.toBeNull();
+
+      const topic = 'persistent://public/default/produce-read-compression';
+      const producer = await client.createProducer({
+        topic,
+        sendTimeoutMs: 30000,
+        batchingEnabled: true,
+        compressionType: 'ZSTD',

Review comment:
       Ah, it seems that sending a message fails when the compression type is set to SNAPPY. I think this is because the Docker image for building the DEB package doesn't include the SNAPPY library.
   https://github.com/apache/pulsar/blob/master/pulsar-client-cpp/pkg/rpm/Dockerfile
   
   So we shouldn't add a test for SNAPPY yet.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] savearray2 commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
savearray2 commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490790233



##########
File path: tests/end_to_end.test.js
##########
@@ -376,5 +376,53 @@ const Pulsar = require('../index.js');
       await reader.close();
       await client.close();
     });
+
+    test('Produce/Read (Compression)', async () => {
+      const client = new Pulsar.Client({
+        serviceUrl: 'pulsar://localhost:6650',
+        operationTimeoutSeconds: 30,
+      });
+      expect(client).not.toBeNull();
+
+      const topic = 'persistent://public/default/produce-read-compression';
+      const producer = await client.createProducer({
+        topic,
+        sendTimeoutMs: 30000,
+        batchingEnabled: true,
+        compressionType: 'ZSTD',

Review comment:
       I can see if I can update the docker container and include the tests.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#issuecomment-694700191


   Please resolve the conflict.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r490842089



##########
File path: tests/end_to_end.test.js
##########
@@ -376,5 +376,53 @@ const Pulsar = require('../index.js');
       await reader.close();
       await client.close();
     });
+
+    test('Produce/Read (Compression)', async () => {
+      const client = new Pulsar.Client({
+        serviceUrl: 'pulsar://localhost:6650',
+        operationTimeoutSeconds: 30,
+      });
+      expect(client).not.toBeNull();
+
+      const topic = 'persistent://public/default/produce-read-compression';
+      const producer = await client.createProducer({
+        topic,
+        sendTimeoutMs: 30000,
+        batchingEnabled: true,
+        compressionType: 'ZSTD',

Review comment:
       Created a pull-request to the apache/pulsar repository.
   https://github.com/apache/pulsar/pull/8086




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-node] massakam commented on a change in pull request #121: Includes Support for ZSTD and SNAPPY Compression

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #121:
URL: https://github.com/apache/pulsar-client-node/pull/121#discussion_r491006106



##########
File path: pulsar-version.txt
##########
@@ -1 +1 @@
-2.5.0
+2.6.1

Review comment:
       Yes. I think the C++ client used for testing should be the lowest version that this library works with.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org