pulsar-client-node icon indicating copy to clipboard operation
pulsar-client-node copied to clipboard

feature: allow messages and producers to handle int64 sequences

Open alanhoff opened this issue 5 years ago • 4 comments

  • 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

alanhoff avatar Sep 27 '20 11:09 alanhoff

@hrsakai I've rebased my PR and it's ready for review 👍

alanhoff avatar Oct 04 '20 12:10 alanhoff

@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();
})();

hrsakai avatar Oct 06 '20 01:10 hrsakai

@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, but it may start losing precision for ints above 253 - 1 (or lower than the negative equivalent). Example:

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 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 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.

alanhoff avatar Dec 26 '20 12:12 alanhoff

@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.

hrsakai avatar Jan 14 '21 02:01 hrsakai