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/03/19 12:38:13 UTC
[GitHub] [pulsar-client-node] yosiat opened a new pull request #83: Consumer
with message listener dies after some time
yosiat opened a new pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83
This PR:
- Moves message listener to consumer
- Use a single consumer instance
The issue I am solving can be reproduced here:
```
const Pulsar = require('pulsar-client');
const readline = require('readline');
const rl = new readline.createInterface({
input: process.stdin,
output: process.stdout
})
function prompt() {
return new Promise((resolve) => {
rl.question(`press any-key to close`, () => resolve());
})
}
(async () => {
// Create a client
const client = new Pulsar.Client({
serviceUrl: 'pulsar://localhost:6650'
});
// Create a consumer
const consumer = await client.subscribe({
topic: 'persistent://public/default/my-topic',
subscription: 'sub1',
subscriptionType: 'Shared',
ackTimeoutMs: 10000,
listener: function(msg) {
console.log(msg.getData().toString());
consumer.acknowledge(msg);
}
});
await prompt();
await consumer.close();
await client.close();
})();
```
This code after some-time will close it's a connection, after this PR it works.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] massakam merged pull request #83: Consumer
with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
massakam merged pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] massakam commented on a change in pull request
#83: Consumer with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#discussion_r399184975
##########
File path: src/Consumer.cc
##########
@@ -45,8 +46,48 @@ void Consumer::Init(Napi::Env env, Napi::Object exports) {
constructor.SuppressDestruct();
}
+struct MessageListenerProxyData {
+ Consumer *consumer;
+ pulsar_message_t *cMessage;
Review comment:
`cMessage` should be declared before `consumer`. Otherwise, you will get warnings when compiling.
```
../src/Consumer.cc:51:21: warning: ‘MessageListenerProxyData::cMessage’ will be initialized after [-Wreorder]
pulsar_message_t *cMessage;
^
../src/Consumer.cc:50:13: warning: ‘Consumer* MessageListenerProxyData::consumer’ [-Wreorder]
Consumer *consumer;
^
../src/Consumer.cc:53:3: warning: when initialized here [-Wreorder]
MessageListenerProxyData(pulsar_message_t *cMessage, Consumer *consumer)
^
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat edited a comment on issue #83: Consumer
with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat edited a comment on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-603287205
@hrsakai @sijie can you please look at this PR?
I would appreciate your help here.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on issue #83: Consumer with
message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-604954020
@massakam thanks for taking a look in this PR.
I am sorry, but the script I posted in the PR doesn't reproduce the issue as you said and this is not the original one. The original one got removed by git stash.
The script that reproduces the issue, is by removing `consumer.close()` and `client.close()`. once you remove them, after couple of seconds you will get:
```
2020-03-27 14:31:45.423 WARN ConsumerImpl:99 | [persistent://public/default/my-topic, sub1, 0] Destroyed consumer which was not properly closed
2020-03-27 14:31:45.423 INFO ClientConnection:1349 | [127.0.0.1:51220 -> 127.0.0.1:6650] Connection closed
2020-03-27 14:31:45.423 INFO ClientConnection:235 | [127.0.0.1:51220 -> 127.0.0.1:6650] Destroyed connection
```
Update the code in the description and pushed a fix for the suggested changes.
Sorry again.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on a change in pull request
#83: Consumer with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on a change in pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#discussion_r399207845
##########
File path: src/Consumer.cc
##########
@@ -22,6 +22,7 @@
#include "Message.h"
#include "MessageId.h"
#include <pulsar/c/result.h>
+#include <iostream>
Review comment:
Removed, used it for debugging.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on issue #83: Consumer with
message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-603287205
@hrrsakai @sijie can you please look at this PR?
I would appreciate your help here.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] massakam commented on issue #83: Consumer with
message listener dies after some time
Posted by GitBox <gi...@apache.org>.
massakam commented on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-605759055
@yosiat I was able to reproduce this issue. Thank you.
And I confirmed that the issue did not occur in the modified code.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] massakam commented on issue #83: Consumer with
message listener dies after some time
Posted by GitBox <gi...@apache.org>.
massakam commented on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-604946741
This change doesn't seem problematic, but I couldn't reproduce the issue in the master code...
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] massakam commented on a change in pull request
#83: Consumer with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#discussion_r399186086
##########
File path: src/Consumer.cc
##########
@@ -22,6 +22,7 @@
#include "Message.h"
#include "MessageId.h"
#include <pulsar/c/result.h>
+#include <iostream>
Review comment:
This seems unnecessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on a change in pull request
#83: Consumer with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on a change in pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#discussion_r394994930
##########
File path: src/Consumer.cc
##########
@@ -45,6 +45,37 @@ void Consumer::Init(Napi::Env env, Napi::Object exports) {
constructor.SuppressDestruct();
}
+struct MessageListenerProxyData {
+ Consumer *consumer;
+ pulsar_message_t *cMessage;
+
+ MessageListenerProxyData(pulsar_message_t *cMessage, Consumer *consumer)
+ : cMessage(cMessage), consumer(consumer) {}
+};
+
+void MessageListenerProxy(Napi::Env env, Napi::Function jsCallback, MessageListenerProxyData *data) {
+ Napi::Object msg = Message::NewInstance({}, data->cMessage);
+ Consumer *consumer = data->consumer;
+ delete data;
+
+ // I haven't figured out how to pass consumer as an argument to method
+ // opened issue here - https://github.com/nodejs/node-addon-api/issues/686
Review comment:
I haven't figured out how to pass consumer as an argument to method.
Since the consumer is `Napi::ObjectWrap` and I need `Napi::Object`.
not sure how to solve this, I solved this in my code like this:
```
const consumer = await client.subscribe({
topic: 'persistent://public/default/my-topic',
subscription: 'sub1',
subscriptionType: 'Shared',
ackTimeoutMs: 10000,
listener: function(msg) {
console.log(msg.getData().toString());
consumer.acknowledge(msg);
}
});
```
referencing the consumer I created in the function.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on a change in pull request
#83: Consumer with message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on a change in pull request #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#discussion_r399207657
##########
File path: src/Consumer.cc
##########
@@ -45,8 +46,48 @@ void Consumer::Init(Napi::Env env, Napi::Object exports) {
constructor.SuppressDestruct();
}
+struct MessageListenerProxyData {
+ Consumer *consumer;
+ pulsar_message_t *cMessage;
Review comment:
Didn't saw those errors because I worked with `npm run build:debug`, now I see them and fixed.
Thanks.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on issue #83: Consumer with
message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-604550699
Did some changes in this PR so I can pass consumer as argument.
But it one test is crashing when closing consumer, I am checking this.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-node] yosiat commented on issue #83: Consumer with
message listener dies after some time
Posted by GitBox <gi...@apache.org>.
yosiat commented on issue #83: Consumer with message listener dies after some time
URL: https://github.com/apache/pulsar-client-node/pull/83#issuecomment-604554560
@sijie re-wrote this a bit, so:
- Call `Ref()` when we a have message listener, to make sure this reference is kept alive.
- I can pass the consumer as a method argument.
----------------------------------------------------------------
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
With regards,
Apache Git Services