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

[RIP-37] New and unified APIs #3973

Closed
aaron-ai opened this issue Mar 14, 2022 · 6 comments
Closed

[RIP-37] New and unified APIs #3973

aaron-ai opened this issue Mar 14, 2022 · 6 comments
Labels
progress/wip Work in progress. Accept or refused to be continue. type/rip
Milestone

Comments

@aaron-ai
Copy link
Member

This issue aims to track the RIP-37, which try to unify a universal APIs spec for RocketMQ.

What can we benefit from this RIP?

  • The behavior of interfaces is more clearly defined, making it easier for newcomers to get started, and the behavior of various interfaces is more predictable.
  • The new APIs unbinds the implementation and interface to realize real interface-oriented programming, and the upgrade cost for users will be lower in the future.
  • The implementation of polyglot clients has always been a difficult problem, and the lack of a unified specification is one of the important reasons. New specifications allow them to be standardized so that users do not behave inconsistently by switching languages.

or you can view more details from the docs link: https://shimo.im/docs/m5kv92OeRRU8olqX

@dugenkui03
Copy link
Contributor

@aaron-ai Some optimized suggestion about api-module, please take a look.

  1. Create masker interface ErrorClassification, and replace ErrorCode with ErrorClassification in ClientException. I think developer need custom error type, such as limitation;
  2. Replace CompletableFuture with Future in Producer#sendAsync, since Future is more extensibie. the default implementation of Producer#sendAsync still can use CompletableFuture;
  3. Replace BackoffRetryPolicy with RetryPolicy in ProducerBuilder#setRetryPolicy .

Describe in Chinese

您好,关于 api-module 有些建议请评估是否采纳:

  1. 创建标记接口ErrorClassification、并使ErrorCode实现该接口,ClientExceptionErrorCode替换为ErrorClassification。因为我认为开发者经常需要自定义异常和异常类型,例如限流,而现在的接口设计其实也支持用户自定义异常;
  2. Producer#sendAsync中使用Future替换CompletableFuture,因为前者已经可以满足接口需要,我们可以再具体实现中在返回CompletableFuture
  3. ProducerBuilder#setRetryPolicy中使用RetryPolicy替换 BackoffRetryPolicy

@aaron-ai
Copy link
Member Author

aaron-ai commented Apr 8, 2022

@aaron-ai Some optimized suggestion about api-module, please take a look.

  1. Create masker interface ErrorClassification, and replace ErrorCode with ErrorClassification in ClientException. I think developer need custom error type, such as limitation;
  2. Replace CompletableFuture with Future in Producer#sendAsync, since Future is more extensibie. the default implementation of Producer#sendAsync still can use CompletableFuture;
  3. Replace BackoffRetryPolicy with RetryPolicy in ProducerBuilder#setRetryPolicy .

Describe in Chinese

您好,关于 api-module 有些建议请评估是否采纳:

  1. 创建标记接口ErrorClassification、并使ErrorCode实现该接口,ClientExceptionErrorCode替换为ErrorClassification。因为我认为开发者经常需要自定义异常和异常类型,例如限流,而现在的接口设计其实也支持用户自定义异常;
  2. Producer#sendAsync中使用Future替换CompletableFuture,因为前者已经可以满足接口需要,我们可以再具体实现中在返回CompletableFuture
  3. ProducerBuilder#setRetryPolicy中使用RetryPolicy替换 BackoffRetryPolicy

Thanks for your suggestion, here is my opinion:

  1. Yes, as you said, error code may be added frequently, what's more, the new APIs is intend to unify both of RemotingCommand-based client and gRPC-based client [RIP-39] Support gRPC protocol #3949 , we need a looser structure to define error code here, so the existing design may be updated in subsequent pull request, welcome to be involved in the code review :)
  2. CompletableFuture may be essential, which allows users add their callback easily.
  3. In most case, BackoffRetryPolicy is enough, so we are not sure about that whether it is time to replace BackoffRetryPolicy by RetryPolicy now, a customized RetryPolicy which is not well-designed may be harmful to the server side, which is we are worried about. The design of BackoffRetryPolicy is refer to gRPC, and has undergone a long-term practice.

@dugenkui03
Copy link
Contributor

dugenkui03 commented Apr 8, 2022

Thanks for your reply, there is already a pr for these suggestion #4134. If some work is already done, please ignored #4134.

CompletableFuture may be essential, which allows users add their callback easily.

Return Future will not stop developers use callback in CompletableFuture because CompletableFuture is a implementation of Future. but return a concret CompletableFuture will affect the extensibility of the interface —— You must accept both pros and cons, such as async timeout, and it is hard to extend CompletableFuture.

In most case, BackoffRetryPolicy is enough.

If BackoffRetryPolicy could satify developers, I think RetryPolicy will be redundant. The reason I think RetryPolicy is better is that Developers will always want to change maxAttempts.

Describe in Chinese

感谢回复,针对这些建议当前已经有了一个pr、见#4134,如果有相关的代码已经实现或者正在进行中则请忽略该PR。

CompletableFuture may be essential, which allows users add their callback easily.

返回Future并不影响开发者在后续处理中使用CompletableFuture中提供的回调方法、因为开发者可以在具体的实现中返回Futrue的实现类CompletableFuture。但是返回具体的类将会影响该接口的可扩展性、也意味着我们必须同时接受CompletableFuture的优缺点、例如异步超时机制在jdk1.9才出现、而该特性对于超时发送也许会很重要;

In most case, BackoffRetryPolicy is enough.

如果BackoffRetryPolicy已经能够应对各种场景那RetryPolicy的定义似乎有些多余?
我认为RetryPolicy使用比较恰当的原因是开发者总是想动态的修改maxAttempts(比如通过redis或者公司各种配置平台配置系统的一些关键参数)。nextAttemptDelay同理。

@aaron-ai
Copy link
Member Author

aaron-ai commented Apr 8, 2022

返回 Future 并不影响开发者在后续处理中使用 CompletableFuture 中提供的回调方法、因为开发者可以在具体的实现中返回 Futrue 的实现类 CompletableFuture

Could you provide an example to add callback or convert Future to CompletableFuture if there is a method for message publishing like this?

Future<SendReceipt> sendAsync(Message message);

About the RetryPolicy, actually it is not public, which means it is not exposed to users, maxAttempts could also be modified in BackoffRetryPolicy, but we may reconsider that if it is reasonable enough to keep RetryPolicy in this module.

@dugenkui03
Copy link
Contributor

dugenkui03 commented Apr 8, 2022

Not convert Future to CompletableFuture, but implements #sendAsync(Message) which return CompletableFuture.

Describe in Chinese

并非将Future转换成CompletableFuture,而是在Producer中实现#sendAsync(Message)方法的时候将返回结果修改为CompletableFuture甚至Future任意的实现,例如返回 guava-ListenableFuture 以使用jdk8的CompletableFuture中没有的特性。像这种在具体实现中在返回具体的类、以扩展或者限定接口能力的方式,在一些比较常见的类库都有使用。

public interface Producer {

    ......

    Future<SendReceipt> sendAsync(Message message);

}

public class DefaultProducer {

    ......

    CompletableFuture<SendReceipt> sendAsync(Message message){
               // 具体的实现,其逻辑的确依赖 CompletableFuture
           ......
    }

}

@aaron-ai
Copy link
Member Author

aaron-ai commented Apr 8, 2022

Actually we considered about ListenableFuture, unfortunately it doesn't belong to the standard library. On the other side user will use Producer#sendAsync to publish message directly rather than DefaultProducer#sendAsync in their programs, so the implementation is not the scope of APIs discussion.

@ShannonDing ShannonDing unpinned this issue May 30, 2022
@lizhiboo lizhiboo added this to the 5.0.0 milestone Sep 6, 2022
@lizhiboo lizhiboo closed this as completed Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
progress/wip Work in progress. Accept or refused to be continue. type/rip
Projects
None yet
Development

No branches or pull requests

4 participants