Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Move queue operations to after debug message so that tenses of message match reality.#818

Closed
jasonimercer wants to merge 1 commit intoros:kinetic-develfrom
jasonimercer:Subscription-Queue-Full-Debug-Tense
Closed

Move queue operations to after debug message so that tenses of message match reality.#818
jasonimercer wants to merge 1 commit intoros:kinetic-develfrom
jasonimercer:Subscription-Queue-Full-Debug-Tense

Conversation

@jasonimercer
Copy link
Copy Markdown
Contributor

Before this change a queue with a limit of 1 element would report the following debug message:

Incoming queue full for topic "baz". Discarding oldest message (current queue size [0])

which implies that the queue is full when it holds 0 elements and that it will discard a message even without elements.

After this change you'll get the message:

Incoming queue full for topic "baz". Discarding oldest message (current queue size [1])

(low-pri PR, quality of life only)

@dirk-thomas

…e match reality.

Before this change a queue with a limit of 1 element would report the following debug message:

Incoming queue full for topic "baz".  Discarding oldest message (current queue size [0])

which implies that the queue is full when it holds 0 elements.
@dirk-thomas
Copy link
Copy Markdown
Member

I am not sure if changing the order makes the message much clearer. I would read it as events in temporal order and with that the current number makes sense:

  1. the incoming queue is full
  2. the oldest message is discarded
  3. the queue size is now N

@jasonimercer
Copy link
Copy Markdown
Contributor Author

If you are happy with the existing message then I'm OK with that. I found it difficult to interpret without looking at the code. I'm OK with this PR getting declined.

@dirk-thomas
Copy link
Copy Markdown
Member

Maybe the message can be improved to make it clear to everyone what it means. Can you suggest a rephrased message which is clear to you?

@jasonimercer
Copy link
Copy Markdown
Contributor Author

The following message could be used in place of moving the queue logic around.

Incoming queue was full for topic "baz". Discarded oldest message (current queue size [0])

@dirk-thomas
Copy link
Copy Markdown
Member

Thanks. I have updated the message with your suggestions: c710fbb

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants