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

[Bug] Global flow control is not work in MessageTransferTask #461

Closed
lrhkobe opened this issue Jul 23, 2021 · 1 comment
Closed

[Bug] Global flow control is not work in MessageTransferTask #461

lrhkobe opened this issue Jul 23, 2021 · 1 comment
Labels
bug Something isn't working
Milestone

Comments

@lrhkobe
Copy link
Contributor

lrhkobe commented Jul 23, 2021

Bug Report

Describe the bug

The global flow control is not work,because the current logic exist problem.

The wrong way:

public void run() {
	...
	if (!rateLimiter.tryAcquire(TRY_PERMIT_TIME_OUT, TimeUnit.MILLISECONDS)) {
		doFlowControlHandle();
		return;
	}

	doSend();
	...
}

In this way, the send thread releases the token immediately after obtaining it.

The right way:

public void run() {
	...
	if (rateLimiter.tryAcquire(TRY_PERMIT_TIME_OUT, TimeUnit.MILLISECONDS)) {
		doSend();
	}else{
		doFlowControlHandle();
		return;
	}

	...
}

Current in MessageTransferTask.java:

public void run() {
	...
	if (!cmd.equals(RESPONSE_TO_SERVER) && !eventMeshTCPServer.getRateLimiter().tryAcquire(TRY_PERMIT_TIME_OUT, TimeUnit.MILLISECONDS)) {
		msg.setHeader(new Header(replyCmd, OPStatus.FAIL.getCode(), "Tps overload, global flow control", pkg.getHeader().getSeq()));
		ctx.writeAndFlush(msg).addListener(
				new ChannelFutureListener() {
					@Override
					public void operationComplete(ChannelFuture future) throws Exception {
						Utils.logSucceedMessageFlow(msg, session.getClient(), startTime, taskExecuteTime);
					}
				}
		);
		logger.warn("======Tps overload, global flow control, rate:{}! PLEASE CHECK!========", eventMeshTCPServer.getRateLimiter().getRate());
		return;
	}

	doSend();
	...
}

Current code in EventMesh is same with the wrong way.

@lrhkobe lrhkobe added the bug Something isn't working label Jul 23, 2021
@ruanwenjun
Copy link
Member

@lrhkobe This two ways may be the same. It seems that the RateLimiter will release tokens at a rate of a given number.

@xwm1992 xwm1992 added this to the 1.3.0 milestone Dec 16, 2021
@xwm1992 xwm1992 changed the title Global flow control is not work in MessageTransferTask [Bug] Global flow control is not work in MessageTransferTask Dec 16, 2021
xwm1992 pushed a commit to xwm1992/EventMesh that referenced this issue Dec 27, 2021
* modify:optimize flow control in downstreaming msg

* modify:optimize stategy of selecting session in downstream msg

* modify:optimize msg downstream,msg store in session

* modify:fix bug:not a @sharable handler

* modify:downstream broadcast msg asynchronously

* modify:remove unneccessary interface in eventmesh-connector-api

* modify:fix conflict

* modify:add license in EventMeshAction

* modify:fix global flow control problem

close apache#461
xwm1992 pushed a commit that referenced this issue Aug 4, 2022
* modify:optimize flow control in downstreaming msg

* modify:optimize stategy of selecting session in downstream msg

* modify:optimize msg downstream,msg store in session

* modify:fix bug:not a @sharable handler

* modify:downstream broadcast msg asynchronously

* modify:remove unneccessary interface in eventmesh-connector-api

* modify:fix conflict

* modify:add license in EventMeshAction

* modify:fix global flow control problem

close #461
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants