Conversation
WalkthroughThis change refactors several core and test components to make chunk file initialization, verification, and related operations fully asynchronous with cancellation support. It updates method signatures, internal calls, and control flow to use async/await patterns, introduces cancellation tokens, and improves error handling and validation logic across chunk management and redaction services. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test/Service
participant TFChunk as TFChunk
participant File as File System
Test->>+TFChunk: await FromCompletedFile(filename, ..., token)
TFChunk->>+File: Open file async
TFChunk->>TFChunk: await InitCompleted(..., token)
TFChunk->>+TFChunk: await VerifyFileHash(token)
TFChunk->>-File: Read and verify hash
TFChunk-->>-Test: TFChunk instance (when complete)
sequenceDiagram
participant RedactionService
participant TFChunkDb
participant TFChunk
RedactionService->>+TFChunkDb: Validate chunk switch (async, token)
TFChunkDb->>+TFChunk: await FromCompletedFile(newChunkFile, ..., token)
TFChunk->>+File: Open and verify file hash async
TFChunk-->>-TFChunkDb: Validated chunk or error
TFChunkDb-->>-RedactionService: Result (success or failure)
RedactionService-->>Caller: Reply with result
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
j-fuxa
force-pushed
the
async
branch
4 times, most recently
from
137a039 to
8bbcd5c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunk.cs (1)
304-304: Remove unnecessary empty line.- Func<TransformType, IChunkTransformFactory> getTransformFactory, CancellationToken token) - { - - var fileInfo = new FileInfo(_filename); + Func<TransformType, IChunkTransformFactory> getTransformFactory, CancellationToken token) + { + var fileInfo = new FileInfo(_filename);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/EventStore.Core.Tests/TransactionLog/when_opening_existing_tfchunk.cs(1 hunks)src/EventStore.Core.Tests/TransactionLog/when_opening_tfchunk_from_non_existing_file.cs(1 hunks)src/EventStore.Core.Tests/TransactionLog/when_reading_from_a_cached_tfchunk.cs(1 hunks)src/EventStore.Core.Tests/TransactionLog/when_uncaching_a_tfchunk.cs(1 hunks)src/EventStore.Core.Tests/Transforms/TransformTests.cs(4 hunks)src/EventStore.Core/Services/RedactionService.cs(2 hunks)src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunk.cs(8 hunks)src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunkReadSide.cs(1 hunks)src/EventStore.Core/TransactionLog/Chunks/TFChunkDb.cs(6 hunks)src/EventStore.Core/TransactionLog/Chunks/TFChunkManager.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/EventStore.Core.Tests/TransactionLog/when_uncaching_a_tfchunk.cs (1)
src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunk.cs (3)
TFChunk(27-1618)TFChunk(174-192)TFChunk(194-197)
src/EventStore.Core/TransactionLog/Chunks/TFChunkDb.cs (2)
src/EventStore.Core/TransactionLog/Chunks/TFChunkManager.cs (2)
TFChunk(436-448)TFChunk(450-461)src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunk.cs (3)
TFChunk(27-1618)TFChunk(174-192)TFChunk(194-197)
src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunk.cs (5)
src/EventStore.Core/TransactionLog/Chunks/TFChunkDb.cs (3)
ValueTask(59-264)ValueTask(385-385)ValueTask(387-412)src/EventStore.Core/TransactionLog/Chunks/TFChunkManager.cs (10)
ValueTask(49-63)ValueTask(79-126)ValueTask(128-146)ValueTask(148-181)ValueTask(183-223)ValueTask(238-257)ValueTask(259-341)ValueTask(463-482)TFChunk(436-448)TFChunk(450-461)src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunkReadSide.cs (1)
TFChunk(12-719)src/EventStore.Core/TransactionLog/Chunks/TFConsts.cs (1)
TFConsts(4-17)src/EventStore.Core/Util/MD5Hash.cs (1)
MD5Hash(8-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build / noble
- GitHub Check: Build / bookworm-slim
- GitHub Check: Build / ubuntu-24.04
- GitHub Check: Build / ubuntu-24.04-arm
🔇 Additional comments (15)
src/EventStore.Core.Tests/TransactionLog/when_uncaching_a_tfchunk.cs (1)
38-40: LGTM: Correctly awaiting the async FromCompletedFile method.The change properly awaits the asynchronous
TFChunk.FromCompletedFilemethod, which now returnsValueTask<TFChunk>instead ofTFChunk. This aligns with the broader async refactoring of chunk initialization methods.src/EventStore.Core.Tests/TransactionLog/when_opening_existing_tfchunk.cs (1)
23-25: LGTM: Correctly awaiting the async FromCompletedFile method.The change properly awaits the asynchronous
TFChunk.FromCompletedFilemethod, maintaining consistency with the async refactoring across the codebase.src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunkReadSide.cs (1)
189-189: LGTM: Correctly using the encapsulated private field.The change from
Chunk.MidpointsDepthtoChunk._midpointsDepthreflects the encapsulation improvement where the field was made private and renamed to follow C# naming conventions for private fields.src/EventStore.Core.Tests/TransactionLog/when_reading_from_a_cached_tfchunk.cs (1)
38-40: LGTM: Correctly awaiting the async FromCompletedFile method.The change properly awaits the asynchronous
TFChunk.FromCompletedFilemethod, ensuring consistent async handling across the test suite.src/EventStore.Core.Tests/TransactionLog/when_opening_tfchunk_from_non_existing_file.cs (2)
10-10: LGTM: Improved class naming to follow C# conventions.The class name change from lowercase to PascalCase follows proper C# naming conventions for class names.
15-17: LGTM: Correctly updated assertion for async method.The change from
Assert.ThrowstoAssert.ThrowsAsyncis necessary becauseTFChunk.FromCompletedFileis now asynchronous. The async lambda properly awaits the method call to test that the expected exception is thrown.src/EventStore.Core/TransactionLog/Chunks/TFChunkManager.cs (1)
305-307: LGTM! Async conversion implemented correctly.The synchronous call to
TFChunk.FromCompletedFilehas been properly converted to async with await, and the cancellation token is correctly passed through.src/EventStore.Core.Tests/Transforms/TransformTests.cs (1)
5-5: LGTM! Test methods properly converted to async.The test has been correctly updated to support the async chunk verification:
- Added necessary using directive for threading
- Properly converted
VerifyChecksumsto asyncValueTaskwith cancellation support- Correctly awaits the async
VerifyFileHashmethodAlso applies to: 56-56, 67-67, 85-85
src/EventStore.Core/TransactionLog/Chunks/TFChunkDb.cs (2)
94-100: LGTM! Async chunk loading implemented correctly.The synchronous chunk loading calls have been properly converted to async:
- All
FromCompletedFileandFromOngoingFilecalls correctly use await- Cancellation tokens are properly propagated
- Good simplification using method group reference
TransformManager.GetFactoryForExistingChunkinstead of lambdaAlso applies to: 108-109, 117-123, 170-176, 206-207
231-260: LGTM! Background verification properly converted to async.The background chunk verification has been correctly updated:
- Uses async delegate with proper cancellation token propagation
- Correctly awaits the async
VerifyFileHashmethod- Maintains the same error handling logic
src/EventStore.Core/TransactionLog/Chunks/TFChunk/TFChunk.cs (3)
90-90: Good encapsulation improvement.Changing
MidpointsDepthfrom public to private field_midpointsDepthimproves encapsulation and follows proper naming conventions for private fields.Also applies to: 185-185
199-217: LGTM! Async initialization methods implemented correctly.The factory methods and initialization methods have been properly converted to async:
FromCompletedFilecorrectly returnsValueTask<TFChunk>InitCompletedproperly handles async operations with cancellation- All async calls use await appropriately
- Cancellation tokens are propagated correctly
Also applies to: 301-359
606-632: Excellent async conversion and optimization.The
VerifyFileHashmethod has been properly converted to async with two improvements:
- Added cancellation support with proper token checking
- Replaced manual byte array comparison with more efficient
MemoryExtensions.SequenceEqualsrc/EventStore.Core/Services/RedactionService.cs (2)
19-56: Good architectural refactoring.The refactoring to split the service into an abstract base class and a generic derived class improves the design and allows for better type safety with the generic
TStreamIdparameter.
206-323: LGTM! Robust async validation implementation.The
IsValidSwitchChunkRequestmethod has been properly converted to async with excellent improvements:
- Uses
Result<TFChunk, SwitchChunkResult>for proper error handling- Comprehensive validation checks for file safety and chunk integrity
- Proper async file stream usage with
await using- Correctly propagates cancellation token to
TFChunk.FromCompletedFile
Signed-off-by: JOSE FUXA <9057699+j-fuxa@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Convert VerifyFileHash and FromCompletedFile to async