Re: Filter problems in async way
Feel free to submit a pull request against the master branch, and ask @chickenlj to review. I happen to know he's working on this area too. Thanks, -Ian. On Thu, Mar 14, 2019 at 7:18 PM 雷舜宇 wrote: > If you have a plan, please let me know in time, I will solve it as soon as > possible, I will always follow up > > Ian Luo 于2019年3月14日周四 下午3:51写道： > > > Maple, we will consider it in 2.7.2 release. > > > > Thanks, > > -Ian. > > > > On Wed, Mar 13, 2019 at 9:57 AM 雷舜宇 wrote: > > > > > issues address:https://github.com/apache/incubator-dubbo/issues/3483 > > > > > > Do you have any opinion on this question, do you need to add a new > field, > > > or do you have any better suggestions? > > > > > >
Re: Filter problems in async way
If you have a plan, please let me know in time, I will solve it as soon as possible, I will always follow up Ian Luo 于2019年3月14日周四 下午3:51写道： > Maple, we will consider it in 2.7.2 release. > > Thanks, > -Ian. > > On Wed, Mar 13, 2019 at 9:57 AM 雷舜宇 wrote: > > > issues address:https://github.com/apache/incubator-dubbo/issues/3483 > > > > Do you have any opinion on this question, do you need to add a new field, > > or do you have any better suggestions? > > >
Re: Filter problems in async way
Maple, we will consider it in 2.7.2 release. Thanks, -Ian. On Wed, Mar 13, 2019 at 9:57 AM 雷舜宇 wrote: > issues address:https://github.com/apache/incubator-dubbo/issues/3483 > > Do you have any opinion on this question, do you need to add a new field, > or do you have any better suggestions? >
Re: Filter problems in async way
Since filter may execute its methods in different threads, first of all we
cannot use thread local to pass the information. Because of this, this
leads us no room but method parameters, in this case, they are invoker and
invocation, to store and extract the extra info when necessary. Based on
this, it looks to me attachment is a reasonable solution.
What we need to think further if timeoutFilter's TIMEOUT_FILTER_START_TIME
needs to be transferred on the wire. If not, we may need to consider how we
can avoid of the transfer but still be able to fetch it in onResponse().
Any thoughts on this?
-Ian.
On Mon, Feb 18, 2019 at 9:30 PM yuhang xiu wrote:
> Hi,ian
>
> Several reasons:
> 1. It seems that the attachment is based on the convention. We need to put
> the information other than the rpc parameter into the attachment. We need
> to ensure that the information and the temporary information (start in the
> timeoutFilter) do not overlap.
> 2. The attachment field may be null. This allows us to make some additional
> judgments and processing before we can use the attachment field:
>
> if (invocation.getAttachments() != null) {
> long start = System.currentTimeMillis();
> invocation.getAttachments().put(TIMEOUT_FILTER_START_TIME,
> String.valueOf(start));
> } else {
> if (invocation instanceof RpcInvocation) {
> RpcInvocation invc = (RpcInvocation) invocation;
> long start = System.currentTimeMillis();
> invc.setAttachment(TIMEOUT_FILTER_START_TIME,
> String.valueOf(start));
> }
> }
>
> I am still thinking about this issue recently. It seems that the extra
> tempAttachment needs to increase the cost of the transmission.
> So I have some hesitation about using tempAttachment. How do you think
> about this problem?
>
> Ian Luo 于2019年2月18日周一 下午6:26写道：
>
> > Yuhang,
> >
> > Why it's a bad idea to use invocation#attachments to store the temp data?
> >
> > Thanks,
> > -Ian.
> >
> >
> > On Fri, Feb 15, 2019 at 2:55 PM yuhang xiu wrote:
> >
> > > Hi, everyone
> > >
> > > Recently, I found that dubbo does not handle the data generated during
> > the
> > > call process when it is asynchronous.
> > >
> > > For example, in TimeoutFilter, we used invocation#attachments to hold
> the
> > > data that needs to be passed (start time in TimeoutFilter). But I don't
> > > think this way is good, although using Invocation.attachments to pass
> > data
> > > does not pollute RpcContext.
> > >
> > > Perhaps a better way is to add a temporary attachment to the Invocation
> > > that only maintains the temporary data generated during the call.
> > >
> > > In addition, I found that we still have a few Filters that are
> currently
> > > problematic in asynchronous scenarios, such as:
> > >
> > > * ActiveLimitFilter
> > > * TraceFilter
> > >
> > > We should determine a plan as soon as possible and then proceed to fix
> > > these Filters.
> > >
> >
>
Re: Filter problems in async way
Hi,ian
Several reasons:
1. It seems that the attachment is based on the convention. We need to put
the information other than the rpc parameter into the attachment. We need
to ensure that the information and the temporary information (start in the
timeoutFilter) do not overlap.
2. The attachment field may be null. This allows us to make some additional
judgments and processing before we can use the attachment field:
if (invocation.getAttachments() != null) {
long start = System.currentTimeMillis();
invocation.getAttachments().put(TIMEOUT_FILTER_START_TIME,
String.valueOf(start));
} else {
if (invocation instanceof RpcInvocation) {
RpcInvocation invc = (RpcInvocation) invocation;
long start = System.currentTimeMillis();
invc.setAttachment(TIMEOUT_FILTER_START_TIME,
String.valueOf(start));
}
}
I am still thinking about this issue recently. It seems that the extra
tempAttachment needs to increase the cost of the transmission.
So I have some hesitation about using tempAttachment. How do you think
about this problem?
Ian Luo 于2019年2月18日周一 下午6:26写道：
> Yuhang,
>
> Why it's a bad idea to use invocation#attachments to store the temp data?
>
> Thanks,
> -Ian.
>
>
> On Fri, Feb 15, 2019 at 2:55 PM yuhang xiu wrote:
>
> > Hi, everyone
> >
> > Recently, I found that dubbo does not handle the data generated during
> the
> > call process when it is asynchronous.
> >
> > For example, in TimeoutFilter, we used invocation#attachments to hold the
> > data that needs to be passed (start time in TimeoutFilter). But I don't
> > think this way is good, although using Invocation.attachments to pass
> data
> > does not pollute RpcContext.
> >
> > Perhaps a better way is to add a temporary attachment to the Invocation
> > that only maintains the temporary data generated during the call.
> >
> > In addition, I found that we still have a few Filters that are currently
> > problematic in asynchronous scenarios, such as:
> >
> > * ActiveLimitFilter
> > * TraceFilter
> >
> > We should determine a plan as soon as possible and then proceed to fix
> > these Filters.
> >
>
Re: Filter problems in async way
Yuhang, Why it's a bad idea to use invocation#attachments to store the temp data? Thanks, -Ian. On Fri, Feb 15, 2019 at 2:55 PM yuhang xiu wrote: > Hi, everyone > > Recently, I found that dubbo does not handle the data generated during the > call process when it is asynchronous. > > For example, in TimeoutFilter, we used invocation#attachments to hold the > data that needs to be passed (start time in TimeoutFilter). But I don't > think this way is good, although using Invocation.attachments to pass data > does not pollute RpcContext. > > Perhaps a better way is to add a temporary attachment to the Invocation > that only maintains the temporary data generated during the call. > > In addition, I found that we still have a few Filters that are currently > problematic in asynchronous scenarios, such as: > > * ActiveLimitFilter > * TraceFilter > > We should determine a plan as soon as possible and then proceed to fix > these Filters. >
