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.

Sign up for GitHub

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

Jump to bottom

Dubbo-2013 #2114

Merged
merged 15 commits into from
Jul 25, 2018
Merged

Dubbo-2013 #2114

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6b0bd1a
parameter是一个注解对象，此处调用它的key方法没有实际用处，所以删除这一行代码
tswstarplanet May 28, 2018
cd8e97d
Merge branch 'pr/1'
tswstarplanet Jun 26, 2018
20644bc
fix the potential null pointer exception
tswstarplanet Jun 26, 2018
06a0ca4
Merge pull request #2 from apache/master
tswstarplanet Jul 6, 2018
a4793ba
Merge pull request #3 from apache/master
tswstarplanet Jul 12, 2018
a0bc50d
Merge pull request #4 from apache/master
tswstarplanet Jul 17, 2018
a93a5db
Add the corethreads tag to the complex tab <dubbo:protocol>
tswstarplanet Jul 17, 2018
e8e2a2d
删除不确定是否有问题的issue的提交代码
tswstarplanet Jul 17, 2018
31a9164
delete the unnecessary import
tswstarplanet Jul 17, 2018
967c739
Merge pull request #5 from apache/master
tswstarplanet Jul 19, 2018
6796da2
add a tag "threadpool" to the complex tag "consumer"
tswstarplanet Jul 19, 2018
7e70a68
add a tag "threadpool" to the complex tag "consumer"; add the test code
tswstarplanet Jul 20, 2018
1316623
add a tag "threadpool" to the complex tag "consumer"; note
tswstarplanet Jul 20, 2018
58980c3
fix an error in unit test
tswstarplanet Jul 21, 2018
889060b
add tag 'threadpool', 'corethreads', 'threads', 'queues' to the compl…
tswstarplanet Jul 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConsumerConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public class ConsumerConfig extends AbstractReferenceConfig {
// networking framework client uses: netty, mina, etc.
private String client;

// client thread pool type: cached, fixed, limit, eager
private String threadpool;

@Override
public void setTimeout(Integer timeout) {
super.setTimeout(timeout);
Expand All @@ -56,4 +59,12 @@ public String getClient() {
public void setClient(String client) {
this.client = client;
}

public String getThreadpool() {
return threadpool;
}

public void setThreadpool(String threadpool) {
this.threadpool = threadpool;
}
}
7 changes: 7 additions & 0 deletions dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ConsumerConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,11 @@ public void testClient() throws Exception {
consumer.setClient("client");
assertThat(consumer.getClient(), equalTo("client"));
}

@Test
public void testThreadpool() throws Exception {
ConsumerConfig consumer = new ConsumerConfig();
consumer.setThreadpool("fixed");
assertThat(consumer.getThreadpool(), equalTo("fixed"));
}
}
5 changes: 5 additions & 0 deletions dubbo-config/dubbo-config-spring/src/main/resources/META-INF/dubbo.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,11 @@
<xsd:documentation><![CDATA[ Transporter layer framework: netty mina.... ]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="threadpool" type="xsd:string" use="optional">
<xsd:annotation>
<xsd:documentation><![CDATA[ Consumer threadpool: cached, fixed, limited, eager]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may also need the following two parameters:

                <xsd:attribute name="threads" type="xsd:string" use="optional">
                    <xsd:annotation>
                        <xsd:documentation><![CDATA[ The thread pool size. ]]></xsd:documentation>
                    </xsd:annotation>
                </xsd:attribute>
                <xsd:attribute name="queues" type="xsd:string" use="optional">
                    <xsd:annotation>
                        <xsd:documentation><![CDATA[ The thread pool queue size. ]]></xsd:documentation>
                    </xsd:annotation>
                </xsd:attribute>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And remember to add the same content to the compatible copy:

https://github.com/apache/incubator-dubbo/blob/master/dubbo-config/dubbo-config-spring/src/main/resources/META-INF/compat/dubbo.xsd

<xsd:anyAttribute namespace="##other" processContents="lax"/>
</xsd:extension>
</xsd:complexContent>
Expand Down
11 changes: 7 additions & 4 deletions .../dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@
import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.store.DataStore;
import org.apache.dubbo.common.utils.ExecutorUtil;
import org.apache.dubbo.common.utils.NamedThreadFactory;
import org.apache.dubbo.common.utils.NetUtils;
import org.apache.dubbo.common.utils.*;
import org.apache.dubbo.remoting.Channel;
import org.apache.dubbo.remoting.ChannelHandler;
import org.apache.dubbo.remoting.Client;
Expand Down Expand Up @@ -113,7 +111,12 @@ public AbstractClient(URL url, ChannelHandler handler) throws RemotingException

protected static ChannelHandler wrapChannelHandler(URL url, ChannelHandler handler) {
url = ExecutorUtil.setThreadName(url, CLIENT_THREAD_POOL_NAME);
url = url.addParameterIfAbsent(Constants.THREADPOOL_KEY, Constants.DEFAULT_CLIENT_THREADPOOL);
String key = Constants.DEFAULT_KEY + Constants.HIDE_KEY_PREFIX + Constants.THREADPOOL_KEY;
String threadpool = url.getParameter(key);
if (StringUtils.isNotEmpty(threadpool)) {
url = url.removeParameter(key);
}
url = url.addParameterIfAbsent(Constants.THREADPOOL_KEY, StringUtils.isNotEmpty(threadpool) ? threadpool : Constants.DEFAULT_CLIENT_THREADPOOL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to change the code here?
I think we will have default.threadpool key in automatically.

Copy link
Contributor Author

@tswstarplanet tswstarplanet Jul 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chickenlj Ok, because I found that when ReferenceConfig integrate the configuration of ConsumerConfig, it will add the prefix "default.", so the "threadpool" property of <dubbo:consumer ../> will change to "default.thread" parameter in the URL.
So, I think we have two ways to optimize the issue.
First, take some special process when integrate the "threadpool" property of ConsumerConfig to ReferenceConfig to wipe off the prefix "default."；
Second is just the method that this sumbit uses.
If the second way is not so good, I think we can take the second one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at the getParameter("threadpool") logic in URL, it will: first check "threadpool" and then check "default.threadpool"

https://github.com/apache/incubator-dubbo/blob/0b6e6f3a375d82aab1a1f17b034e666c63f61103/dubbo-common/src/main/java/org/apache/dubbo/common/URL.java#L421

return ChannelHandlers.wrap(handler, url);
}

Expand Down