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

Dubbo 3.2.0 broadcast + injvm not work as expected #12334

Closed
1 task done
CodePlayer opened this issue May 16, 2023 · 4 comments · Fixed by #12347
Closed
1 task done

Dubbo 3.2.0 broadcast + injvm not work as expected #12334

CodePlayer opened this issue May 16, 2023 · 4 comments · Fixed by #12347
Labels
type/bug Bugs to being fixed

Comments

@CodePlayer
Copy link
Contributor

CodePlayer commented May 16, 2023

  • I have searched the issues of this repository and believe that this is not a duplicate.

Environment

  • Dubbo version: 3.2.0 / 3.2.1
  • Operating System version: Windows 10
  • Java version: OpenJDK 17.0.6

Steps to reproduce this issue

public interface RemoteCacheSyncService {

  void flushCache();

}

@DubboService
public class RemoteCacheSyncServiceImpl implements RemoteCacheSyncService {

  @Override
  public void flushCache() {
	  // some code			
  }

}

We have 3 microservices, referred to as A, B, and C.
Each service contains the above code.

But, just C has a @DubboReference to the interface only.

@DubboReference(cluster = ClusterRules.BROADCAST)
RemoteCacheSyncService remoteCacheSyncService;

Expected Behavior

Call ALL implementations in ALL microservices.

Actual Behavior

However, since there is a service implementation of the interface in the same application ( C ),
Dubbo will use the injvm protocol. Only the service implementation located in C will be invoked.

Same issue with #6842 , however, it came back again.

@CodePlayer CodePlayer added the type/bug Bugs to being fixed label May 16, 2023
@AlbumenJ
Copy link
Member

@PhoenixBackendon PTAL

@Phixsura
Copy link
Contributor

I see what you mean. The current solution is configured as forced remote in the settings, which should be able to solve it, or we can provide a rule to adapt to the requirements based on this broadcasting mechanism, and I will finish it later.

AlbumenJ pushed a commit that referenced this issue May 19, 2023
…12347)

* Fix issue #12334.provide broadcast rules to adapt the calling mode.

* add isDebugEnabled
@CodePlayer
Copy link
Contributor Author

CodePlayer commented May 19, 2023

I see what you mean. The current solution is configured as forced remote in the settings, which should be able to solve it, or we can provide a rule to adapt to the requirements based on this broadcasting mechanism, and I will finish it later.

https://github.com/PhoenixBackendon/dubbo/blob/35da167f6a921f9c0c0799b807442f72d0fefaff/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/wrapper/ScopeClusterInvoker.java#L132
logger.debug() should consider use placeholder {} .

@Phixsura
Copy link
Contributor

I see what you mean. The current solution is configured as forced remote in the settings, which should be able to solve it, or we can provide a rule to adapt to the requirements based on this broadcasting mechanism, and I will finish it later.

https://github.com/PhoenixBackendon/dubbo/blob/35da167f6a921f9c0c0799b807442f72d0fefaff/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/wrapper/ScopeClusterInvoker.java#L132 logger.debug() should consider use placeholder {} .

I completely agree with your point. Although the Dubbo logger is configuration-based and not all underlying loggers support placeholder-style value assignment, Dubbo currently only supports logger.debug(String message) instead of logger.debug(String message, Object... params) for logging. I suggest adding a conversion layer in the logger to enable placeholder-style logging, which would better align with elegant logging practices. This improvement would not only enhance code readability and maintainability but also provide greater flexibility and extensibility in logging. I propose that we create an issue to discuss and implement this enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Bugs to being fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants