Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netty: Swap to UniformStreamByteDistributor #11659

Open
forthehell opened this issue Oct 31, 2024 · 4 comments
Open

netty: Swap to UniformStreamByteDistributor #11659

forthehell opened this issue Oct 31, 2024 · 4 comments
Milestone

Comments

@forthehell
Copy link

forthehell commented Oct 31, 2024

Is your feature request related to a problem?

We send the asyncUnaryCall in line in the same thread and with only one channel , we find the rpc call arrived in server is out of order。

Describe the solution you'd like

We find the reason is the WeightedFairQueueByteDistributor 。 And theUniformStreamByteDistributor can more likely do the job, but there is no configuration to switch between those ByteDistributors。 can we add the configuration about it?

if(FAIR_Distributor){
  WeightedFairQueueByteDistributor dist = new WeightedFairQueueByteDistributor(connection);
  dist.allocationQuantum(16 * 1024); // Make benchmarks fast again.
  streamByteDistributor = dist;
}else{
  UniformStreamByteDistributor uniformStreamByteDistributor =  new UniformStreamByteDistributor(connection);
  uniformStreamByteDistributor.minAllocationChunk(16 * 1024);
  streamByteDistributor = uniformStreamByteDistributor;
}

Describe alternatives you've considered

Additional context

@ejona86
Copy link
Member

ejona86 commented Nov 1, 2024

If you start two asynchronous RPCs, there's no guarantees about their orders. The distributor doesn't even guarantee you what you're wanting, as the message size can change the order.

Since priorities were deprecated in RFC 9113, it might be reasonable to just change to UniformStreamByteDistributor unconditionally. We should also look into what's happening with Netty in this space (esp Netty 4.2), as they would have changed their default in the http2 builder, that grpc doesn't use.

So using the uniform distributor can be fair. But it is very bad to do it for your purpose. There are no ordering guarantees unless you get some response from the server before sending another RPC/message.

@forthehell
Copy link
Author

forthehell commented Nov 4, 2024

If you start two asynchronous RPCs, there's no guarantees about their orders. The distributor doesn't even guarantee you what you're wanting, as the message size can change the order.

Since priorities were deprecated in RFC 9113, it might be reasonable to just change to UniformStreamByteDistributor unconditionally. We should also look into what's happening with Netty in this space (esp Netty 4.2), as they would have changed their default in the http2 builder, that grpc doesn't use.

So using the uniform distributor can be fair. But it is very bad to do it for your purpose. There are no ordering guarantees unless you get some response from the server before sending another RPC/message.

I wonder why the message size can change the order ? asynchronous RPCs dataFrame will be sent to the WriterQueue which belonged to the save channel。if flush the WriterQueue command ,they will be comsumed in order 。 if the distributor is fair, then they will be sent in order too。 I mean no ordering guarantees is the design concept, but if using the uniform distributor, it is very bad to do it for my purpose, but actually,it can do it ?

@ejona86
Copy link
Member

ejona86 commented Nov 4, 2024

I wonder why the message size can change the order ?

If the first message is bigger then the second message, then the fair distributor could finish sending the second message first. Whether this actually happens is dependent on the quantum size (16 KiB on that line), but that configuration is highly internal.

Even if they are sent in-order, the server's executor could end up running the second one "first," due to both running concurrently (so which is "first" is dependent on your application logic) or kernel thread scheduling.

Then there's other cases like, "the two RPCs may not end up on the same connection," because of GOAWAY.

@ejona86 ejona86 changed the title send asyncUnaryCall in line netty: Swap to UniformStreamByteDistributor Nov 12, 2024
@ejona86
Copy link
Member

ejona86 commented Nov 12, 2024

This issue will look at swapping to UniformStreamByteDistributor, since priorities are deprecated. We should first understand what is happening in Netty, especially Netty 4.2. But our decision can be separate from theirs.

(The goal of this change is not to preserve the order of RPCs. That can be a side-effect, but not the goal)

@ejona86 ejona86 added this to the Next milestone Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants