Skip to content

Conversation

@TomasEkeli
Copy link
Contributor

@TomasEkeli TomasEkeli commented Jun 19, 2025

This should help with issue #1273 - intermittent exceptions from WorkflowLoggingService.

There is no concurrent HashSet in .NET, so we use ConcurrentDictionary with a byte as the value to simulate a set. For this use-case we do not care if the Key already exists, so we can use TryAdd to add items without considering the return value.

Description

Replaces HashSet<string> as the collection that stores Activity and Workflow names for logging with ConcurrentDictionary<string, byte>

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1273

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@TomasEkeli TomasEkeli requested review from a team as code owners June 19, 2025 07:00
@TomasEkeli TomasEkeli force-pushed the Use_concurrent_collections_in_WorkflowLoggingService branch from 5bb541e to b795ca2 Compare June 19, 2025 07:06
@WhitWaldo
Copy link
Contributor

First of all, thank you for taking the time to spot this and submit a PR - I really appreciate it!

Could you please sign your commit? You can read more about DCO requirement here, but we'll need it in place before the PR can be merged.

WhitWaldo
WhitWaldo previously approved these changes Jun 20, 2025
Copy link
Contributor

@WhitWaldo WhitWaldo left a comment

Choose a reason for hiding this comment

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

Looks fine to me - good to go once the DCO is in place.

@TomasEkeli
Copy link
Contributor Author

TomasEkeli commented Jun 20, 2025

I have force-pushed with the requried string, but for some reason the DCO does not register it?

edit: I have check the documetation you linked, and it only seems to require adding a "Signed-off-by" -line to the commit. I did that yesterday through a force-push, but the DCO check is still red.

Does it have to be re-activated or something?

@WhitWaldo
Copy link
Contributor

I have force-pushed with the requried string, but for some reason the DCO does not register it?

edit: I have check the documetation you linked, and it only seems to require adding a "Signed-off-by" -line to the commit. I did that yesterday through a force-push, but the DCO check is still red.

Does it have to be re-activated or something?

Looks like it's failing because the email address you're using in GitHub doesn't match the one you signed off with. If you can either add the other one to GitHub or change the DCO to match, we'll be square here.

@TomasEkeli
Copy link
Contributor Author

Ah, I understand. I have many e-mail addresses, and I'm not 100% sure which github thinks I should use.

I'll force-push a new one with a likely address :)

@TomasEkeli TomasEkeli force-pushed the Use_concurrent_collections_in_WorkflowLoggingService branch from 3070b43 to 00726e2 Compare June 22, 2025 15:21
This should help with issue dapr#1273 - intermittent exceptions from
WorkflowLoggingService.

There is no concurrent HashSet in .NET, so we use ConcurrentDictionary
with a byte as the value to simulate a set. For this use-case we do not
care if the Key already exists, so we can use TryAdd to add items
without considering the return value.

Signed-off-by: Tomas Ekeli <[email protected]>
@TomasEkeli TomasEkeli force-pushed the Use_concurrent_collections_in_WorkflowLoggingService branch from 00726e2 to 78aba15 Compare June 22, 2025 18:14
Copy link
Contributor

@WhitWaldo WhitWaldo left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@WhitWaldo WhitWaldo merged commit e982b1c into dapr:master Jun 23, 2025
12 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in v1.16 Release Tracking Board Jun 23, 2025
@WhitWaldo
Copy link
Contributor

Thank you very much for your contribution here!

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

intermittent exceptions from WorkflowLoggingService

2 participants