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/27 11:09:52 UTC
[GitHub] [pulsar-client-node] alanhoff opened a new pull request #127: feature: allow messages and producers to handle int64 sequences
alanhoff opened a new pull request #127:
URL: https://github.com/apache/pulsar-client-node/pull/127
* Allows the initial sequence ID to be passed as a string: `client.createProducer({initialSequenceId: '1234'})` so JS can send 64 bits integers encoded as string.
* Allows the message sequence ID to be passed as a string `producer.send({sequenceId: '1234'})` so JS can send 64 bits integers encoded as string.
* Implements `Producer.getLastSequenceId` which returns `Napi::String` since `Napi::Number` cannot be used for 64 bits integers.
* Closes #124
----------------------------------------------------------------
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] alanhoff commented on pull request #127: feature: allow messages and producers to handle int64 sequences
Posted by GitBox <gi...@apache.org>.
alanhoff commented on pull request #127:
URL: https://github.com/apache/pulsar-client-node/pull/127#issuecomment-703246623
@hrsakai I've rebased my PR and it's ready for review 👍
----------------------------------------------------------------
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] alanhoff commented on pull request #127: feature: allow messages and producers to handle int64 sequences
Posted by GitBox <gi...@apache.org>.
alanhoff commented on pull request #127:
URL: https://github.com/apache/pulsar-client-node/pull/127#issuecomment-751349897
@hrsakai @sijie sorry for the delay here, let's start by correcting some of my misconceptions:
JS do support ints higher than 32 bits when representing them as a [`Number`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER), but it *may* start losing precision for ints above 2<sup>53</sup> - 1 (or lower than the negative equivalent). Example:
```javascript
const x = Number.MAX_SAFE_INTEGER + 1;
const y = Number.MAX_SAFE_INTEGER + 2;
console.log(x === y); // will output true
```
That said, we're "kinda" safe at the moment since it's the native addon that's [incrementing the sequence](https://github.com/apache/pulsar/blob/1cf29f244a783f62aa6e4b6860135758e0f8034c/pulsar-client-cpp/lib/ProducerImpl.cc#L65) using a proper `int64_t`, however IMHO I don't think the JS client should be using the `Number` primitive to represent an `int64_t` since that's unsafe.
The correct thing to do would be changing all sequences so they accept a [BigInt](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt) instead of a Number, but it would be a breaking change and it wouldn't support Node.js < `10.4.0`. So in the meanwhile this PR allows devs to optionally pass sequences as strings which are being casted into `int64_t` by the addon without losing precision.
----------------------------------------------------------------
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] hrsakai commented on pull request #127: feature: allow messages and producers to handle int64 sequences
Posted by GitBox <gi...@apache.org>.
hrsakai commented on pull request #127:
URL: https://github.com/apache/pulsar-client-node/pull/127#issuecomment-703972525
@alanhoff
I can't reproduce the issue.
I set 5294967295(greater than 32 bits integers) to initialSequenceId, but sequence id looks correct.
Please tell me how to reproduce.
```
initialSequenceId = 5294967295
lastSequenceId = 5294967395
```
code
```
const Pulsar = require('../index.js');
(async () => {
// Create a client
const client = new Pulsar.Client({
serviceUrl: 'pulsar://localhost:6650',
operationTimeoutSeconds: 30,
});
// Create a producer
const producer = await client.createProducer({
topic: 'persistent://public/default/my-topic',
sendTimeoutMs: 30000,
batchingEnabled: false,
initialSequenceId: 5294967295,
});
console.log('initialSequenceId = ' + producer.getLastSequenceId());
// Send messages
for (let i = 0; i < 100; i += 1) {
const msg = `my-message-${i}`;
await producer.send({
data: Buffer.from(msg),
});
}
console.log('lastSequenceId = ' + producer.getLastSequenceId());
await producer.close();
await client.close();
})();
```
----------------------------------------------------------------
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] hrsakai commented on pull request #127: feature: allow messages and producers to handle int64 sequences
Posted by GitBox <gi...@apache.org>.
hrsakai commented on pull request #127:
URL: https://github.com/apache/pulsar-client-node/pull/127#issuecomment-759885824
@alanhoff
Thank you for your explanation.
Could you change initialSequenceId to accept NUMBER and BigInt and getLastSequenceId() to return BigInt?
We can accept that we don't support `Node.js < 10.4.0`.
----------------------------------------------------------------
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