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

Limit Dead Letter logging #6482

Merged
merged 1 commit into from
May 2, 2023
Merged

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented May 1, 2023

Pull Request Description

Dead Letter logging is occasionally flooding our logs which is confusing to users reporting bugs. Left the possibility of a single report so that we know that something is happening.

@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label May 1, 2023
@hubertp hubertp force-pushed the wip/hubert/limit-dead-letter-logging branch from 3c9e368 to 0503fe2 Compare May 1, 2023 12:30
Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are dead letters? Found a blog. Where are such messages originating from? Our code or code or imported libraries? Any reference to examples of such messages?

@hubertp
Copy link
Contributor Author

hubertp commented May 2, 2023

What are dead letters? Found a blog. Where are such messages originating from? Our code or code or imported libraries? Any reference to examples of such messages?

Messages sent to actors that are no longer up and running. We often have short-lived actors that are stopped after serving the request.
Ideally, we would like to eliminate such messages but they are also harmless. That's why I limited it to 1 to occasionally try to debug it.

Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose randomly some recent log, and I can see that there are roughly 5000 dead letter trace messages. Good thing that we get rid of those, since they are garbage anyway.

@hubertp hubertp added CI: Ready to merge This PR is eligible for automatic merge CI: Clean build required CI runners will be cleaned before and after this PR is built. labels May 2, 2023
Dead Letter logging is ocassionally flooding our logs which is confusing
to users reporting bugs. Left the possibility of a single report so that
we know that something is happening.
@hubertp hubertp force-pushed the wip/hubert/limit-dead-letter-logging branch from 0503fe2 to 708c480 Compare May 2, 2023 12:02
@mergify mergify bot merged commit ce4ecc6 into develop May 2, 2023
@mergify mergify bot deleted the wip/hubert/limit-dead-letter-logging branch May 2, 2023 14:00
@4e6 4e6 mentioned this pull request May 2, 2023
3 tasks
mergify bot pushed a commit that referenced this pull request May 2, 2023
@JaroslavTulach
Copy link
Member

Messages sent to actors that are no longer up and running. We often have short-lived actors that are stopped after serving the request. ... they are also harmless.

Are they harmless? I am envisioning an agent that transfers money and handles withdraw, but "forgets" to add the money to another account.

5000 dead letter trace messages. ... since they are garbage anyway

I can't imagine why someone would be sending 5000 messages to a dead actor and be satisfied they get ignored?

However I may have a vision I have in my mind that doesn't match reality.

Procrat added a commit that referenced this pull request May 3, 2023
* develop: (34 commits)
  Continued Execution Context work and some little fixes (#6506)
  IDE's logging to a file (#6478)
  Fix application config (#6513)
  Cloud/desktop mode switcher (#6448)
  Fix doubled named arguments bug (#6422)
  Reimplement `enso_project` as a proper builtin (#6352)
  Fix layer ordering between dropdown and breadcrumbs backgrounds.  (#6483)
  Multiflavor layers (#6477)
  DataflowAnalysis preserves dependencies order (#6493)
  Implement `create_database_table` for Database Table (#6467)
  Limit Dead Letter logging (#6482)
  More reliable shutdown of the EnsoContext to save resources (#6468)
  Make execution mode `live` default for CLI (#6496)
  Finishing Vector Editor (#6470)
  Proper handling of multiple list views. (#6461)
  Fix disappearing cached shapes (#6458)
  Add Execution Context control to Text.write (#6459)
  Change defaults for `Connection.tables` and ensure that `Connection.query` recognizes all available tables (#6443)
  Introducing @BuiltinMethod.needsFrame and InlineableNode (#6442)
  Hide profile button behind a feature flag (#6430)
  ...
Procrat added a commit that referenced this pull request May 4, 2023
* develop:
  Fix cut-off in text visualisations (#6421)
  Infer correct synthetic name for nested modules (#6525)
  Delete unused websocket dependency (#6535)
  Run typecheck and eslint on `./run lint` (#6314)
  Force pending saves if client closes abruptly (#6514)
  Continued Execution Context work and some little fixes (#6506)
  IDE's logging to a file (#6478)
  Fix application config (#6513)
  Cloud/desktop mode switcher (#6448)
  Fix doubled named arguments bug (#6422)
  Reimplement `enso_project` as a proper builtin (#6352)
  Fix layer ordering between dropdown and breadcrumbs backgrounds.  (#6483)
  Multiflavor layers (#6477)
  DataflowAnalysis preserves dependencies order (#6493)
  Implement `create_database_table` for Database Table (#6467)
  Limit Dead Letter logging (#6482)
  More reliable shutdown of the EnsoContext to save resources (#6468)
  Make execution mode `live` default for CLI (#6496)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants