[On Hold] Feature steam improvements and Extract streaming from GAgentBase.#99
[On Hold] Feature steam improvements and Extract streaming from GAgentBase.#99
Conversation
arg-foo
left a comment
There was a problem hiding this comment.
Please add logs to important sections of the code. The code is currently lacking useful logging in case of issues.
arg-foo
left a comment
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Same as above, lock or disable reentrant
| return groupIndex; | ||
| } | ||
|
|
||
| public async Task UnregisterChildAsync(GrainId childGrainId) |
There was a problem hiding this comment.
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 = '.'; |
There was a problem hiding this comment.
Is this GAgentNamespaceSeparator serving any purpose?
change MaxChildrenPerGroup to 1500
No description provided.