Skip to content

[On Hold] Feature steam improvements and Extract streaming from GAgentBase.#99

Open
arg-foo wants to merge 16 commits intodevfrom
feature-steam-auric
Open

[On Hold] Feature steam improvements and Extract streaming from GAgentBase.#99
arg-foo wants to merge 16 commits intodevfrom
feature-steam-auric

Conversation

@arg-foo
Copy link
Contributor

@arg-foo arg-foo commented Apr 8, 2025

No description provided.

@arg-foo arg-foo changed the title Feature steam improvements Feature steam improvements and Extract streaming from GAgentBase. Apr 8, 2025
Copy link
Contributor Author

@arg-foo arg-foo left a comment

Choose a reason for hiding this comment

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

Please add logs to important sections of the code. The code is currently lacking useful logging in case of issues.

Copy link
Contributor Author

@arg-foo arg-foo left a comment

Choose a reason for hiding this comment

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

LGTM. Will need to first do load testing to verify that this PR is working as intended.

return Task.FromResult(State.ParentGrainId);
}

public async Task<int> RegisterChildAsync(GrainId childGrainId)
Copy link
Contributor

Choose a reason for hiding this comment

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

This method needs disable reentrant or apply locking on children count. Would it be better to add unit test?

return groupIndex;
}

public async Task<int> RegisterManyChildAsync(List<GrainId> childrenGrainIds)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, lock or disable reentrant

return groupIndex;
}

public async Task UnregisterChildAsync(GrainId childGrainId)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need multiple different configuration/configuration classes that contains the same value with same/different purposes?

{
public const string StreamProvider = "Aevatar";
public const string DefaultStreamNamespace = "Aevatar";
public const char GAgentNamespaceSeparator = '.';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this GAgentNamespaceSeparator serving any purpose?

@codecov
Copy link

codecov bot commented Apr 29, 2025

Codecov Report

Attention: Patch coverage is 72.03390% with 99 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/Aevatar.Core/GAgentBase.cs 44.11% 36 Missing and 2 partials ⚠️
...evatar.Core/EventPublish/StreamCoordinatorGrain.cs 83.90% 25 Missing and 3 partials ⚠️
...ar.Core/EventPublish/EventPubChildrenGroupGrain.cs 68.65% 20 Missing and 1 partial ⚠️
...atar.Core/Extensions/EventWrapperBaseExtensions.cs 60.00% 4 Missing and 4 partials ⚠️
...vatar.Core.Abstractions/AevatarCoreStreamConfig.cs 42.85% 3 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
src/Aevatar.Core.Abstractions/StateBase.cs 100.00% <100.00%> (ø)
src/Aevatar.Core/Extensions/StreamExtensions.cs 100.00% <100.00%> (ø)
src/Aevatar.Core/GAgentBase.Observers.cs 72.01% <100.00%> (-0.38%) ⬇️
src/Aevatar.Core/GAgentBase.Publish.cs 65.51% <100.00%> (+7.18%) ⬆️
src/Aevatar.Core/GAgentBase.Subscribe.cs 39.13% <100.00%> (-43.77%) ⬇️
src/Aevatar.Core/GAgentBase.SyncWorker.cs 66.66% <100.00%> (ø)
src/Aevatar/AevatarModule.cs 100.00% <100.00%> (ø)
src/Aevatar/Extensions/OrleansHostExtensions.cs 41.17% <100.00%> (+3.67%) ⬆️
...vatar.Core.Abstractions/AevatarCoreStreamConfig.cs 42.85% <42.85%> (ø)
...atar.Core/Extensions/EventWrapperBaseExtensions.cs 60.00% <60.00%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@eanzhao eanzhao changed the title Feature steam improvements and Extract streaming from GAgentBase. [On Hold] Feature steam improvements and Extract streaming from GAgentBase. May 15, 2025
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.

Performance bottle neck on single parent propagating events to increasing children

4 participants