You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "chickenlj (GitHub)" <gi...@apache.org> on 2019/03/01 09:36:23 UTC

[GitHub] [incubator-dubbo] chickenlj opened issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Recently, I am thinking of making the whole invoker chain inside Dubbo asynchronous, event-driven. And I believe that `Filter` can be changed to better adapt to the new model.

Before 2.7.0, you can find the definition of Filter was completely synchronous:

```java
public interface Filter {
    Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
       // before
        Result result = invoker.invoke(invocation);
       // after
      return result;
    }
}
```

In 2.7.0, a callback `onResponse` was added to allow users to handle the async value when result returns.

```java
public interface Filter {
   Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
   }

   void onResponse(Result result, Invoker<?> invoker, Invocation invocation) {
        // biz return
    }
}
```

Now, how about the following definition, totally asynchronous and event-driven?

```java
public interface Filter {
   void onSend(Invocation invocation) {
     // before invoke, throw exception to terminate 
    }

   void onResponse(Result result, Invoker<?> invoker, Invocation invocation) {
        // biz return successfully
    }

   void onError(Throwable e) throws RpcException{
        // biz throw exception
    }
}
```

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] chickenlj commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "chickenlj (GitHub)" <gi...@apache.org>.
@carryxyh Let's wait to hear more voices.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] carryxyh commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@chickenlj 
If we can reach a consensus, can you let me try it?
:)

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] carryxyh commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
About compatibility issues.

The current version will enter `onResponse` due to all requests (correct or incorrect). If we modify the code, the correct response enters `onResponse`, the error response enters `onError`, and `onResponse` will only process the correct request if the user does not modify the `Filter` code.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] chickenlj commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Bringing Ian's question to here:
>In your new proposed interface, how could we construct a filter chain? Say,
how could filter-a process further a value processed by filter-b?

I don't think we still need a nested Filter chain as before, the new Filter can simply work as follow:

```java
Iterate(filters) {
    filter.onSend();
}

Result result = invoker.invoke();

result.whenComplete((value, t) {
  Iterate tail-to-head(filters) {
     filter.doResponse();
     filter.doError();
  }
});
```                                                                                   

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "kezhenxu94 (GitHub)" <gi...@apache.org>.
I'm wondering:
- whether the proposed definition will result to the [Callback Hell](https://en.wiktionary.org/wiki/callback_hell), many programming languages tried to avoid **Callback Hell** such as `Future` in Java and `Promise` in Javascript
- and the proposed definition seems make it difficult to synchronous calls (in case that users need it)

Can we borrow the design from The Netty Project, say returning a `org.apache.dubbo.filter.Future` that is declared like below:

```java
class Future<T> {
  T get(); // wait for the result synchronously
  void onComplete(SomeContext c); // asynchronously
  void onSend(SomeContext c); // asynchronously
  void onError(SomeContext c); // asynchronously
  // ... other callbacks
}
```

then users can do something like this:
```java
Filter filter = ...;
Future f = filter.invoke(...);
// if synchronous call is needed
f.get();
// if asynchronous call is needed
f.onSend(context -> { /* lambda */ });
f.onError(context -> { /* lambda */ });
// ... others
```

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "kezhenxu94 (GitHub)" <gi...@apache.org>.
Can we borrow the design of The Netty Project, say returning a `org.apache.dubbo.filter.Future` that is declared like below:

```java
class Future<T> {
  T get(); // wait for the result synchronously
  void onComplete(SomeContext c); // asynchronously
  void onSend(SomeContext c); // asynchronously
  void onError(SomeContext c); // asynchronously
  // ... other callbacks
}
```

then users can do something like this:
```java
Filter filter = ...;
Future f = filter.invoke(...);
// if synchronous call is needed
f.get();
// if asynchronous call is needed
f.onSend(context -> { /* lambda */ });
f.onError(context -> { /* lambda */ });
// ... others
```

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] carryxyh commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
About compatibility issues.

The current version will enter `onResponse` due to all requests (correct or incorrect). If we modify the code, the correct response enters `onResponse`, the error response enters `onError`, and `onResponse` will only process the correct request if the user does not modify the `Filter` code.

Is this a problem that we should consider?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "kezhenxu94 (GitHub)" <gi...@apache.org>.
I'm wondering:
- whether the proposed definition will result to the [Callback Hell](https://en.wiktionary.org/wiki/callback_hell), many programming languages tried to avoid **Callback Hell** such as `Future` in Java and `Promise` in Javascript
- and the proposed definition seems make it difficult to synchronous calls (in case that users need it, let's leave it possible for synchronous calls)

Can we borrow the design from The Netty Project, say returning a `org.apache.dubbo.filter.Future` that is declared like below:

```java
class Future<T> {
  T get(); // wait for the result synchronously
  void onComplete(SomeContext c); // asynchronously
  void onSend(SomeContext c); // asynchronously
  void onError(SomeContext c); // asynchronously
  // ... other callbacks
}
```

then users can do something like this:
```java
Filter filter = ...;
Future f = filter.invoke(...);
// if synchronous call is needed
f.get();
// if asynchronous call is needed
f.onSend(context -> { /* lambda */ });
f.onError(context -> { /* lambda */ });
// ... others
```

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "kezhenxu94 (GitHub)" <gi...@apache.org>.
Can we borrow the design from The Netty Project, say returning a `org.apache.dubbo.filter.Future` that is declared like below:

```java
class Future<T> {
  T get(); // wait for the result synchronously
  void onComplete(SomeContext c); // asynchronously
  void onSend(SomeContext c); // asynchronously
  void onError(SomeContext c); // asynchronously
  // ... other callbacks
}
```

then users can do something like this:
```java
Filter filter = ...;
Future f = filter.invoke(...);
// if synchronous call is needed
f.get();
// if asynchronous call is needed
f.onSend(context -> { /* lambda */ });
f.onError(context -> { /* lambda */ });
// ... others
```

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] carryxyh commented on issue #3585: [DISCUSSION] Make filter totally asynchronous and event-driven.

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
good idea. But I want to remind that in `onError` we still need some information other than `Throwable`, such as `invocation` and `result`.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3585 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org