Add async PerformAsync and FlushAsync methods with CancellationToken support by NehanPathan · Pull Request #1170 · apache/lucenenet

NehanPathan · 2025-08-26T10:19:12Z

@paulirwin for review.

Linked Issue:
Fixes #1169

Description:
This PR implements asynchronous support for the replication framework in Lucene.NET, based on suggestions from @paulirwin. The changes include:

IReplicationService interface
- Added Task PerformAsync(IReplicationRequest request, IReplicationResponse response, CancellationToken cancellationToken = default)
- Supports cancellation and async execution of replication tasks.
IReplicationResponse interface
- Added Task FlushAsync(CancellationToken cancellationToken = default)
- Provides asynchronous flush support to the underlying response stream.
ReplicationService class
- Added PerformAsync method mirroring Perform, using await for I/O operations (Stream.CopyToAsync, WriteAsync, FlushAsync).
- Accepts optional CancellationToken for cooperative cancellation.
SessionToken class
- Added Task SerializeAsync(Stream output, CancellationToken cancellationToken = default) to allow asynchronous serialization.
AspNetCoreReplicationResponse class
- Added FlushAsync method that calls HttpResponse.Body.FlushAsync with optional CancellationToken.

Motivation:

Modern .NET applications require async I/O for scalability and responsiveness.
Existing synchronous methods require AllowSynchronousIO=true in Kestrel, which is not recommended for production.
This PR ensures the replication API follows .NET async best practices without breaking existing synchronous behavior.

Additional Context:

The synchronous APIs (Perform and Flush) remain unchanged for backward compatibility.
This PR improves compatibility for ASP.NET Core hosting scenarios where synchronous I/O is restricted.

…support

paulirwin

@NehanPathan Thanks for the PR! This mostly looks good. A few comments added.

…O, updating PerformAsync, SessionToken, and ASP.NET Core response handling.

NightOwl888

I did a bit of research and it is understandable why this was a bit confusing. Stream doesn't support a high-performance way to write async without allocations. I came across System.IO.Pipelines as a possible solution. It is an official package that supports net462, netstandard2.0, and .NET Core, but is not included in the box with any of them. It maintains a pooled buffer, so all of the actual write operations don't involve any I/O, but also doesn't allocate any non-pooled heap. It will write the buffer to the underlying stream by calling PipeWriter.FlushAsync(). The trick is to figure out a good method for when to call PipeWriter.FlushAsync() without calling it too often or not often enough.

To use it, it is more sensible to make extension methods on the PipeWriter than on Stream.

PipeWriterExtensions - Click to expand

using Lucene.Net.Replicator;
using System;
using System.IO.Pipelines;
using System.Text;

#nullable enable

namespace Lucene.Net.IO
{
    internal static class PipeWriterExtensions
    {
        public static int WriteUTFBigEndian(this PipeWriter writer, string value)
        {
            if (writer is null)
                throw new ArgumentNullException(nameof(writer));
            if (value is null)
                throw new ArgumentNullException(nameof(value));

            ReadOnlySpan<char> valueSpan = value.AsSpan();
            ulong byteLength = CountUTFBytes(valueSpan);
            if (byteLength > ushort.MaxValue)
                throw new EncoderFallbackException("Encoded string too long.");

            int utf8ByteLength = (int)byteLength;
            int offset = 0;

            Span<byte> buffer = writer.GetSpan(utf8ByteLength + 2);
            offset = WriteInt16BigEndianToBuffer(utf8ByteLength, buffer, offset);
            offset = WriteUTFBigEndianBytesToBuffer(valueSpan, utf8ByteLength, buffer, offset);

            writer.Advance(offset);
            return offset;
        }

        public static void WriteInt32BigEndian(this PipeWriter writer, int value)
        {
            if (writer is null)
                throw new ArgumentNullException(nameof(writer));

            Span<byte> buffer = writer.GetSpan(4);
            buffer[0] = (byte)(value >> 24);
            buffer[1] = (byte)(value >> 16);
            buffer[2] = (byte)(value >> 8);
            buffer[3] = (byte)value;
            writer.Advance(sizeof(int));
        }

        public static void WriteInt64BigEndian(this PipeWriter writer, long value)
        {
            if (writer is null)
                throw new ArgumentNullException(nameof(writer));

            Span<byte> buffer = writer.GetSpan(8);
            buffer[0] = (byte)(value >> 56);
            buffer[1] = (byte)(value >> 48);
            buffer[2] = (byte)(value >> 40);
            buffer[3] = (byte)(value >> 32);
            buffer[4] = (byte)(value >> 24);
            buffer[5] = (byte)(value >> 16);
            buffer[6] = (byte)(value >> 8);
            buffer[7] = (byte)value;
            writer.Advance(sizeof(long));
        }

        public static void WriteNullTokenMarker(this PipeWriter writer, SessionToken? token)
        {
            if (writer is null)
                throw new ArgumentNullException(nameof(writer));

            const byte Null = 0, NonNull = 1;

            Span<byte> buffer = writer.GetSpan(1);
            buffer[0] = token is null ? Null : NonNull;
            writer. Advance(1);
        }

        // ========================
        // Helper methods for UTF
        // ========================
        private static ulong CountUTFBytes(ReadOnlySpan<char> value)
        {
            ulong utfCount = 0;
            foreach (char ch in value)
            {
                if (ch > 0 && ch <= 127)
                    utfCount++;
                else if (ch <= 2047)
                    utfCount += 2;
                else
                    utfCount += 3;
            }
            return utfCount;
        }

        private static int WriteInt16BigEndianToBuffer(int value, Span<byte> buffer, int offset)
        {
            buffer[offset++] = (byte)(value >> 8);
            buffer[offset++] = (byte)value;
            return offset;
        }

        private static int WriteUTFBigEndianBytesToBuffer(ReadOnlySpan<char> value, long count, Span<byte> buffer, int offset)
        {
            foreach (char ch in value)
            {
                if (ch > 0 && ch <= 127)
                {
                    buffer[offset++] = (byte)ch;
                }
                else if (ch <= 2047)
                {
                    buffer[offset++] = (byte)(0xc0 | (0x1f & (ch >> 6)));
                    buffer[offset++] = (byte)(0x80 | (0x3f & ch));
                }
                else
                {
                    buffer[offset++] = (byte)(0xe0 | (0x0f & (ch >> 12)));
                    buffer[offset++] = (byte)(0x80 | (0x3f & (ch >> 6)));
                    buffer[offset++] = (byte)(0x80 | (0x3f & ch));
                }
            }
            return offset;
        }
    }
}

ReplicationSerivce.PerformAsync - Click to expand

public virtual Task PerformAsync(
    IReplicationRequest request,
    IReplicationResponse response,
    CancellationToken cancellationToken = default)
{
    return ExecuteReplicationAsync(
        request,
        response,
        stream => stream.CopyToAsync(response.Body, 8192, cancellationToken),
        async token =>
        {
            var writerOptions = new StreamPipeWriterOptions(minimumBufferSize: 8192);
            var writer = PipeWriter.Create(response.Body, writerOptions);

            writer.WriteNullTokenMarker(token);
            if (token is not null)
                await token.SerializeAsync(writer, cancellationToken);
        },
        () => response.FlushAsync(cancellationToken)
    );
}

SessionToken.SerializeAsync - Click to expand

public async Task SerializeAsync(PipeWriter writer, CancellationToken cancellationToken = default)
{
    if (writer is null)
        throw new ArgumentNullException(nameof(writer));
    if (!writer.CanGetUnflushedBytes)
        throw new ArgumentException("PipeWriter implementations that don't allow retrieval of UnflushedBytes are not supported.");

    // Leave a little wiggle room here so the buffer isn't likely to get reallocated.
    const int MaxUnflushedBytes = 7500;

    writer.WriteUTFBigEndian(Id);
    writer.WriteUTFBigEndian(Version);
    writer.WriteInt32BigEndian(SourceFiles.Count);

    foreach (var pair in SourceFiles)
    {
        if (writer.UnflushedBytes >= MaxUnflushedBytes)
            await writer.FlushAsync(cancellationToken).ConfigureAwait(false);

        writer.WriteUTFBigEndian(pair.Key);
        writer.WriteInt32BigEndian(pair.Value.Count);

        foreach (var file in pair.Value)
        {
            if (writer.UnflushedBytes >= MaxUnflushedBytes)
                await writer.FlushAsync(cancellationToken).ConfigureAwait(false);

            writer.WriteUTFBigEndian(file.FileName);
            writer.WriteInt64BigEndian(file.Length);
        }
    }

    await writer.FlushAsync(cancellationToken).ConfigureAwait(false);
}

That passed the tests - or so I thought. When I tried setting a breakpoint in PerformAsync() it was never hit. The reason for this is because the tests depend on either the ReplicationServiceMiddleware or ReplicationServlet class depending on target framework.

I made the following changes to make the ReplicationServiceMiddleware perform async.

ReplicationServiceMiddleware - Click to expand

public class ReplicationServiceMiddleware
{
    private readonly RequestDelegate next;
    private readonly IReplicationService service;

    public ReplicationServiceMiddleware(RequestDelegate next, IReplicationService service)
    {
        this.next = next ?? throw new ArgumentNullException(nameof(next));
        this.service = service ?? throw new ArgumentNullException(nameof(service));
    }

    public async Task InvokeAsync(HttpContext context)
    {
        await service.PerformAsync(context.Request, context.Response, context.RequestAborted);

        // This is a terminating endpoint. Do not call the next delegate/middleware in the pipeline.
    }
}

AspNetCoreReplicationServiceExtentions - Click to expand

public static class AspNetCoreReplicationServiceExtentions
{
    /// <summary>
    /// Extension method that mirrors the signature of <see cref="IReplicationService.Perform"/> using AspNetCore as implementation.
    /// </summary>
    public static void Perform(this IReplicationService self, HttpRequest request, HttpResponse response)
    {
        self.Perform(new AspNetCoreReplicationRequest(request), new AspNetCoreReplicationResponse(response));
    }

    /// <summary>
    /// Extension method that mirrors the signature of <see cref="IReplicationService.PerformAsync"/> using AspNetCore as implementation.
    /// </summary>
    public static async Task PerformAsync(this IReplicationService self, HttpRequest request, HttpResponse response, CancellationToken cancellationToken = default)
    {
        await self.PerformAsync(new AspNetCoreReplicationRequest(request), new AspNetCoreReplicationResponse(response), cancellationToken);
    }
}

Unfortunatly, now it is throwing an exception due to synchronous I/O:

System.InvalidOperationException : Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true.
(Test: Lucene.Net.Replicator.Http.HttpReplicatorTest.TestBasic)
 Stack Trace: 
ResponseBodyWriterStream.Flush()
AspNetCoreReplicationResponse.Flush() line 71
<>c__DisplayClass14_0.<Perform>b__2() line 213
ReplicationService.ExecuteReplicationAsync(IReplicationRequest request, IReplicationResponse response, Func`2 copyStreamFunc, Func`2 writeTokenFunc, Func`1 flushFunc) line 184
ReplicationService.Perform(IReplicationRequest request, IReplicationResponse response) line 196
AspNetCoreReplicationServiceExtentions.Perform(IReplicationService self, HttpRequest request, HttpResponse response) line 34
ReplicationServiceMiddleware.InvokeAsync(HttpContext context) line 96
MockErrorMiddleware.InvokeAsync(HttpContext context) line 159
<<SendAsync>g__RunRequestAsync|0>d.MoveNext()
--- End of stack trace from previous location ---
ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
HttpClientBase.Execute(HttpRequestMessage request) line 207
HttpClientBase.ExecuteGet(String request, String[] parameters) line 200
HttpReplicator.CheckForUpdate(String currentVersion) line 71
ReplicationClient.DoUpdate() line 174
ReplicationClient.UpdateNow() line 463
HttpReplicatorTest.TestBasic() line 111
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

If I uncomment the lines to allow synchronous I/O in ReplicationServiceMiddleware, it then calls the async method, but fails CheckIndex (whether using the StreamExtensions approach or PipeWriterExtensions approach). It is showing an invalid byte count either way. I suspect your recent experience with this probably gives you a leg up to try to address the issues that are causing the failure. Do you have additional tests that you didn't post here?

@paulirwin - Thoughts on using System.IO.Pipelines if we can get the bugs worked out? For the time being we can add the package reference to Lucene.Net.Replicator, but if this ends up needing to be patched through all the way to IndexWriter to function, we will need to add the dependency to Lucene.Net also.

paulirwin · 2025-08-28T14:07:49Z

@paulirwin - Thoughts on using System.IO.Pipelines if we can get the bugs worked out? For the time being we can add the package reference to Lucene.Net.Replicator, but if this ends up needing to be patched through all the way to IndexWriter to function, we will need to add the dependency to Lucene.Net also.

This is currently mostly blocking our GSoC work from wrapping up, so I'd prefer to accept the allocations for now and do a fast-follow item for the pipelines. Assuming that it won't change the API contract either for the replicator interfaces/classes or the across-the-wire data format.

NightOwl888 · 2025-08-28T14:13:54Z

This is currently mostly blocking our GSoC work from wrapping up, so I'd prefer to accept the allocations for now and do a fast-follow item for the pipelines. Assuming that it won't change the API contract either for the replicator interfaces/classes or the across-the-wire data format.

It changes the API contract on SessionToken

public async Task SerializeAsync(Stream output, CancellationToken cancellationToken = default)

to

public async Task SerializeAsync(PipeWriter writer, CancellationToken cancellationToken = default)

Not sure whether that is a deal breaker, though. This method is public, but called by PerformAsync().

paulirwin · 2025-08-28T14:17:34Z

It changes the API contract on SessionToken

Thanks, good to know. Once we get there, we could decide whether just adding an overload is warranted, leaving the Stream version unused by our code. That way it wouldn't be a breaking change. My vote is let's move forward with the Stream approach for now.

paulirwin · 2025-08-28T14:20:11Z

Alternatively, we could make the serialize methods internal and go ahead and accept a breaking change that they're not supposed to be part of our public API? It seems to me like this type is intended to be somewhat opaque anyways, and arguably it's a SRP violation to have serialization inside of it. Just a thought.

NightOwl888 · 2025-08-28T14:52:14Z

Alternatively, we could make the serialize methods internal and go ahead and accept a breaking change that they're not supposed to be part of our public API? It seems to me like this type is intended to be somewhat opaque anyways, and arguably it's a SRP violation to have serialization inside of it. Just a thought.

Good point. Since SerializeAsync() is a brand new method, we can mark it internal until we have it finalized. PerformAsync() is the public method that gets called by the hosted network service, so SerializeAsync() isn't likely something that Lucene.Net.Extensions will need. And if so, there is always InternalsVisibleTo.

…ations in StreamExtensions

NightOwl888

Alright. This is looking good.

All that is left is to port over the tests from J2N for StreamExtensions. But don't let it hold up the show. I know this is blocking the GSoC project, so if this isn't in the cards it should be fine. @paulirwin was kind enough to close many of the gaps in our test coverage, and it would be a shame to create a new one, but we can let this slide if we have to.

…nding J2N-based async tests in TestStreamExtensions

NehanPathan · 2025-08-29T18:53:26Z

@NightOwl888

just a quick update — I had to force push this branch.

Reason: in last commit I guess you removed the Perform() method from ReplicationServlet entirely. I’ve corrected that by commenting it out instead of deleting so the history/context remains clear.

Other changes I pushed:

Added an XML summary for the new PerformAsync() method in the extension.
Cleaned up some duplicate commented code.
Added new async read/write tests (adapted from J2N) — and all test cases are passing. ✅

Please let me know if you notice anything odd or if you’d prefer a different approach.

NightOwl888 · 2025-08-29T19:54:06Z

@NightOwl888

just a quick update — I had to force push this branch.

Reason: in last commit I guess you removed the Perform() method from ReplicationServlet entirely. I’ve corrected that by commenting it out instead of deleting so the history/context remains clear.

Other changes I pushed:

Added an XML summary for the new PerformAsync() method in the extension.

Cleaned up some duplicate commented code.

Added new async read/write tests (adapted from J2N) — and all test cases are passing. ✅

Please let me know if you notice anything odd or if you’d prefer a different approach.

Thanks. Removing Perform() wasn't intentional. And thanks for adding the tests.

Although, I realized that by switching our tests to using async only, we are no longer testing the synchronous methods. I am looking into how to set it up without too much code duplication. I think we pretty much have to duplicate everything in ReplicationServlet. The NewHttpServer() method overloads can get a bool async parameter.

Then comes the tricky bit. I think the HttpReplicatorTest class could be converted into a HttpReplicationServerTestCase that is abstract. Then an abstract bool property could be added named UseAsyncServer, which would be passed into NewHttpServer(). Then we could just make 2 new classes, AsyncHttpReplicatorTest and HttpReplicatorTest that subclass HttpReplicationServerTestCase, each setting the bool value appropriately.

I am about to turn in for the night, but would be happy to work on it tomorrow if you don't get the chance.

NehanPathan · 2025-08-30T12:40:09Z

Thanks. Removing Perform() wasn't intentional. And thanks for adding the tests.

Although, I realized that by switching our tests to using async only, we are no longer testing the synchronous methods. I am looking into how to set it up without too much code duplication. I think we pretty much have to duplicate everything in ReplicationServlet. The NewHttpServer() method overloads can get a bool async parameter.

Then comes the tricky bit. I think the HttpReplicatorTest class could be converted into a HttpReplicationServerTestCase that is abstract. Then an abstract bool property could be added named UseAsyncServer, which would be passed into NewHttpServer(). Then we could just make 2 new classes, AsyncHttpReplicatorTest and HttpReplicatorTest that subclass HttpReplicationServerTestCase, each setting the bool value appropriately.

I am about to turn in for the night, but would be happy to work on it tomorrow if you don't get the chance.

@NightOwl888

Thanks — that makes sense.

Your approach sounds clean and avoids too much duplication. I’m happy to try setting that up now, unless you’d prefer to handle it yourself since you already have the structure in mind.

Either way, I’ll follow your lead — just let me know what you prefer.

Also, one quick thing — after my last push yesterday, I noticed in the checks that 2 tests failed (I also got the email around 5/6 ie github try to re run but it fail). But now everything shows green and all tests are passing. Do you know if there was any specific reason for that? Did you make some changes, or does GitHub sometimes re-run and fix it automatically? I wasn’t fully sure why that happened.

NightOwl888 · 2025-08-30T13:57:29Z

@NightOwl888

Thanks — that makes sense.

Your approach sounds clean and avoids too much duplication. I’m happy to try setting that up now, unless you’d prefer to handle it yourself since you already have the structure in mind.

Either way, I’ll follow your lead — just let me know what you prefer.

I have been working on this, but came to a bit of a fork in the road when it comes to setting it up because of how the different configurations are setup between legacy apps and new. I have a setup that simply does what I said, but it would be much cleaner if there were only 1 NewHttpServer() overload without so many branching paths, so I am attempting to unify the configuration a bit more between older and newer versions of ASP.NET Core.

Anyway, let me work on it a bit more before handing it off to you. I think I can come up with a compromise without digging too deep into the custom configuration territory (where Lucene.Net.Extensions is meant to shine).

Also, one quick thing — after my last push yesterday, I noticed in the checks that 2 tests failed (I also got the email around 5/6 ie github try to re run but it fail). But now everything shows green and all tests are passing. Do you know if there was any specific reason for that? Did you make some changes, or does GitHub sometimes re-run and fix it automatically? I wasn’t fully sure why that happened.

Yeah, I spotted the failures and saw that they were due to server connectivity problems, not actual software failures. So, I restarted the tests and they succeeded the second time.

paulirwin · 2025-08-30T14:40:03Z

so I am attempting to unify the configuration a bit more between older and newer versions of ASP.NET Core.

Versions older than ASP.NET Core 8 are out of support, so if you're referring to <= 7 in use by the existing AspNetCore sample code, I think we should drop support for them. I do not think we need the burden of supporting older, unsupported versions of ASP.NET Core. (Part of the reason why we're looking at moving the extensions into a separate repo.)

NightOwl888 · 2025-08-30T16:10:42Z

so I am attempting to unify the configuration a bit more between older and newer versions of ASP.NET Core.

Versions older than ASP.NET Core 8 are out of support, so if you're referring to <= 7 in use by the existing AspNetCore sample code, I think we should drop support for them. I do not think we need the burden of supporting older, unsupported versions of ASP.NET Core. (Part of the reason why we're looking at moving the extensions into a separate repo.)

Understood. But replicator is both a client and sever-side technology. For simplicity, we are testing both client and server-side on the same target framework using Microsoft.AspNetCore.TestHost. I suppose we could aim to find a way to host the server-side on ASP.NET Core while we test the client side on .NET Framework if we are going to discuss dropping server-side support on all but .NET Core.

Do note there are other server-side HTTP stacks that ReplicationServer could be hosted on that are in support.

ASP.NET (System.Web)
WCF (server-side)
HttpListener
OWIN / Katana

I don't feel strongly about supporting these stacks, but just thought I would point out there are other options for hosting a replication server than on ASP.NET Core. But it doesn't make a lot of sense to test them explicitly (which is why we stuck with Microsoft.AspNetCore.TestHost). We specfically added interfaces to be able to support these as well as future HTTP stacks.

However, it seems like it would be extremely limiting if we didn't allow .NET Framework users to use the client-side replicator components.

…asynchronous APIs as well as both Startup class and Middleware configurations (when both are supported).

paulirwin

Minor feedback added about some comments in the code. @NehanPathan I'm open to approving this without the J2N-ported unit tests if you want to do that in a follow-up PR, or let me know if you are already working on porting those and will have that done soon. Thanks!

NehanPathan · 2025-08-30T19:05:03Z

Minor feedback added about some comments in the code. @NehanPathan I'm open to approving this without the J2N-ported unit tests if you want to do that in a follow-up PR, or let me know if you are already working on porting those and will have that done soon. Thanks!

@paulirwin
I have already added the J2N-ported async tests ie in (src/Lucene.Net.Tests/Support/IO/TestStreamExtensions.cs)
in the last commit, as per Shad’s suggestion.
pls correct me if i misunderstood....
And
Regarding the commented-out code: that is our old synchronous code. We left it commented for future reference while adding the new async code. Shad is currently working on that part only ie separating the testing code so that both synchronous and asynchronous code can be tested independently.

… don't have to rely on Microsoft.AspNetcore.TestHost prior to .NET Core. Bumped Microsoft.AspNetcore.TestHost package to 9.0.8. Refactored tests to separate ASP.NET Core functionality from HttpListener functionality.

…NetCore on .NET Core 8 or higher

…works but net8.0.

…ive dependency System.Text.Encodings.Web, which only existed to bump the package to one without known security vulnerabilities. Bumped Microsoft.AspNetCore.Http.Abstractions to 2.3.0.

NightOwl888

I have pushed a few commits to update the tests for both synchronous and asynchronous I/O.

Please review my changes, but everything before my commits looks pretty good (aside from one issue, mentioned inline).

Also, based on your feedback, I went ahead and had ChatGPT create a TestServer based on System.Net.HttpListener so we can continue testing server functionality on target frameworks older than net8.0 without having to reference old versions of Microsoft.AspNetCore.TestHost. I also dropped all targets from Lucene.Net.Replicator.AspNetCore except for net8.0. But I am sure that whole assembly will be going away soon, anyway.

Since configuring an HTTP server to use ReplicationService is a bit of a black art, the tests were designed not only as tests but also as a demos on a couple of different options for integrating it into ASP.NET Core. However, once the replicator extensions in lucenenet-extensions have matured, we can move all demos over there and do only the simplified HttpListener tests on all target frameworks so we can drop the dependency on Microsoft.AspNetCore.TestHost.

I also spotted a potential issue with adding FlushAsync() to the same interface as Flush() and PerformAsync() on the same interface as Perform(). See my inline comment for details.

NehanPathan · 2025-09-01T14:41:16Z

just a quick note — I accidentally pulled in apache:master while trying to update my branch (git pull --rebase origin async-replication). That created an unintended merge commit (54d355c).

I’ve since reverted it by resetting the branch (git reset --hard HEAD~1) and force-pushing, so the branch is now clean and only contains the intended async replication changes. Sorry for the noise!

…tionService, Middleware, and TestServer to support async replication with conditional sync/async DI.

NightOwl888

Well, crap. I totally missed the fact that both Flush() and FlushAsync() are simply delegating the call to the public property Body on the same interface. So, rather than splitting IReplicationService into 2 interfaces, it is more sensible to remove Flush() and FlushAsync() from the interface and change all callers to do the operation directly on the IReplicationService. Body.

The simplest way back is to revert 349621f and then remove Flush() and FlushAsync() from IReplicationResponse, then fix all of the compile errors.

Since IReplicationService is something we own and isn't likely something a user will need to implement, we can also just revert it to include both overloads, also.

… ReplicationService, Middleware, and TestServer to support async replication with conditional sync/async DI." This reverts commit 349621f.

…rmAsync, update ReplicationServlet, Middleware, and TestServer to use single interface

NightOwl888

This looks good to me.

@paulirwin - you may want to make another pass just to make sure I didn't miss anything.

paulirwin · 2025-09-02T13:06:13Z

Congrats @NehanPathan on this PR and thanks for the feedback cycles!

NehanPathan · 2025-09-02T13:24:16Z

@paulirwin @NightOwl888

Thank you so much, for your guidance, trust, and feedback throughout this! I really appreciate it.😊

Add async PerformAsync and FlushAsync methods with CancellationToken …

8f28cd7

…support

NightOwl888 requested a review from paulirwin August 26, 2025 11:07

paulirwin requested changes Aug 26, 2025

View reviewed changes

Comment thread src/dotnet/Lucene.Net.Replicator.AspNetCore/AspNetCoreReplicationResponse.cs Outdated

Comment thread src/Lucene.Net.Replicator/SessionToken.cs Outdated

Comment thread src/Lucene.Net.Replicator/Http/ReplicationService.cs Outdated

Refactor replication service and stream extensions for fully async I/…

3e1068a

…O, updating PerformAsync, SessionToken, and ASP.NET Core response handling.

NightOwl888 reviewed Aug 28, 2025

View reviewed changes

Comment thread src/Lucene.Net/Support/IO/StreamExtensions.cs Outdated

Comment thread src/Lucene.Net.Replicator/Http/ReplicationService.cs

NightOwl888 self-requested a review August 28, 2025 14:20

NightOwl888 requested changes Aug 28, 2025

View reviewed changes

refactor: rename BigEndian methods and use ArrayPool for buffer alloc…

406940f

…ations in StreamExtensions

NightOwl888 requested changes Aug 29, 2025

View reviewed changes

feat: Add async read/write support in ReplicationServlet and correspo…

0276e10

…nding J2N-based async tests in TestStreamExtensions

NehanPathan force-pushed the async-replication branch from 477d2d3 to 0276e10 Compare August 29, 2025 18:44

Add XML Summary For PerformAsync and remove dupilcate comment code

8415457

Lucene.Net.Tests.Replicator: Refactored to test both synchronous and …

4cace1f

…asynchronous APIs as well as both Startup class and Middleware configurations (when both are supported).

paulirwin reviewed Aug 30, 2025

View reviewed changes

Comment thread src/Lucene.Net.Tests.Replicator/Http/ReplicationServlet.cs Outdated

Comment thread src/Lucene.Net.Tests.Replicator/Http/ReplicationServlet.cs Outdated

NightOwl888 added 4 commits August 31, 2025 13:29

Lucene.Net.Tests.Replicator: Only reference Lucene.Net.Replicator.Asp…

df85266

…NetCore on .NET Core 8 or higher

Lucene.Net.Replicator.AspNetCore: Dropped support for all target fram…

e057d33

…works but net8.0.

Lucene.Net.Replicator.AspNetCore: Removed PackageReference to transit…

4a06a5d

…ive dependency System.Text.Encodings.Web, which only existed to bump the package to one without known security vulnerabilities. Bumped Microsoft.AspNetCore.Http.Abstractions to 2.3.0.

NightOwl888 self-requested a review August 31, 2025 07:55

NightOwl888 reviewed Aug 31, 2025

View reviewed changes

Comment thread src/Lucene.Net.Replicator/Support/Http/Abstractions/IReplicationResponse.cs Outdated

NehanPathan force-pushed the async-replication branch from 54d355c to 4a06a5d Compare September 1, 2025 14:39

Add IAsyncReplicationServer/IAsyncReplicationResponse, update Replica…

349621f

…tionService, Middleware, and TestServer to support async replication with conditional sync/async DI.

NightOwl888 requested changes Sep 1, 2025

View reviewed changes

NehanPathan added 2 commits September 2, 2025 06:22

Revert "Add IAsyncReplicationServer/IAsyncReplicationResponse, update…

8dcdc0d

… ReplicationService, Middleware, and TestServer to support async replication with conditional sync/async DI." This reverts commit 349621f.

Revert async interfaces, unify IReplicationService with Perform/Perfo…

869ed37

…rmAsync, update ReplicationServlet, Middleware, and TestServer to use single interface

NehanPathan mentioned this pull request Sep 2, 2025

Feature : Add TODO for Perform Method and XML Documentation Comments paulirwin/lucenenet-extensions#5

Merged

NightOwl888 approved these changes Sep 2, 2025

View reviewed changes

paulirwin approved these changes Sep 2, 2025

View reviewed changes

paulirwin merged commit f38de47 into apache:master Sep 2, 2025
276 checks passed

paulirwin added this to the 4.8.0-beta00018 milestone Sep 2, 2025

NehanPathan deleted the async-replication branch September 2, 2025 13:24

Uh oh!

Conversation

NehanPathan commented Aug 26, 2025

Uh oh!

paulirwin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NightOwl888 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paulirwin commented Aug 28, 2025

Uh oh!

NightOwl888 commented Aug 28, 2025

Uh oh!

paulirwin commented Aug 28, 2025

Uh oh!

paulirwin commented Aug 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NightOwl888 commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NightOwl888 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NehanPathan commented Aug 29, 2025

Uh oh!

NightOwl888 commented Aug 29, 2025

Uh oh!

NehanPathan commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NightOwl888 commented Aug 30, 2025

Uh oh!

paulirwin commented Aug 30, 2025

Uh oh!

NightOwl888 commented Aug 30, 2025

Uh oh!

paulirwin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NehanPathan commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NightOwl888 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NehanPathan commented Sep 1, 2025

Uh oh!

NightOwl888 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NightOwl888 left a comment

paulirwin left a comment •

edited

Loading

NightOwl888 commented Aug 28, 2025 •

edited

Loading

NehanPathan commented Aug 30, 2025 •

edited

Loading

NehanPathan commented Aug 30, 2025 •

edited

Loading