Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix loosing message on consumer #86

Merged
merged 21 commits into from
May 4, 2023
Merged

Conversation

gpad
Copy link
Contributor

@gpad gpad commented Apr 14, 2023

An example is this running, where the hello355 is missing.

2023-04-14T10:55:05.523Z debug: 	{
  magicVersion: 80,
  chunkType: 0,
  numEntries: 2,
  numRecords: 2,
  timestamp: 1681469702859n,
  epoch: 1n,
  chunkFirstOffset: 354n,
  chunkCrc: -246489329,
  dataLength: 34,
  trailerLength: 0,
  reserved: 0,
  messageType: 0,
  messageLength: 13
} 
2023-04-14T10:55:05.523Z debug: 	on deliver -> DeliverResponse {
  response: {
    size: 87,
    key: 8,
    version: 1,
    subscriptionId: 0,
    messages: [ <Buffer 68 65 6c 6c 6f 33 35 34> ]
  }
} - consumers: 1 
[ 'hello354' ]

As you can see the numEntries: 2, and numRecords: 2, should describe this chunk contains 2 messages when usually we receive numEntries: 1 and numRecords: 1

2023-04-14T10:55:05.523Z debug: 	{
  magicVersion: 80,
  chunkType: 0,
  numEntries: 1,
  numRecords: 1,
  timestamp: 1681469702860n,
  epoch: 1n,
  chunkFirstOffset: 356n,
  chunkCrc: 2039626224,
  dataLength: 17,
  trailerLength: 0,
  reserved: 0,
  messageType: 0,
  messageLength: 13
} 
2023-04-14T10:55:05.523Z debug: 	on deliver -> DeliverResponse {
  response: {
    size: 70,
    key: 8,
    version: 1,
    subscriptionId: 0,
    messages: [ <Buffer 68 65 6c 6c 6f 33 35 36> ]
  }
} - consumers: 1 
[ 'hello356' ]

@gpad gpad requested review from ggllcu and l4mby April 14, 2023 08:54
@gpad
Copy link
Contributor Author

gpad commented Apr 14, 2023

The implementation of this PR is able to consume 1000 messages if executed with .only in the describe. If executed with all other test seems like there is a sort of leak of stream ...

@gpad gpad mentioned this pull request Apr 27, 2023
@l4mby l4mby mentioned this pull request May 2, 2023
@albertobarrila albertobarrila force-pushed the fix/loosing-message-on-consumer branch from 6b8ca0d to e33096a Compare May 2, 2023 13:31
src/connection.ts Outdated Show resolved Hide resolved
test/e2e/metadata_update.test.ts Outdated Show resolved Hide resolved
src/util.ts Outdated Show resolved Hide resolved
test/e2e/connect.test.ts Outdated Show resolved Hide resolved
test/e2e/connect.test.ts Outdated Show resolved Hide resolved
test/e2e/credit.test.ts Show resolved Hide resolved
@gpad gpad marked this pull request as ready for review May 3, 2023 20:15
test/e2e/declare_consumer.test.ts Outdated Show resolved Hide resolved
test/e2e/credit.test.ts Show resolved Hide resolved
@gpad gpad merged commit 5fa31a1 into main May 4, 2023
@gpad gpad deleted the fix/loosing-message-on-consumer branch May 4, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants