Skip to content

Conversation

jiafu1115
Copy link
Contributor

Motivation:

JedisConnectionFactory should support setting clientName into Jedis so
that we can trouble shooting easier by client name.

now no client name be showed for client list:
id=18 addr=10.140.201.34:51637 fd=12 name= age=85650 idle=85619 flags=N
db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 obl=0 oll=0 omem=0
events=r cmd=scan

Modifications:

add private member: clientName and use it in constructor for Jedis Pool.

Result:

easier to trouble shooting with connection name:

id=2 addr=10.224.38.23:33652 fd=5 name=sentinel-164682c3-cmd age=1350882
idle=1 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 obl=0 oll=0
omem=0 events=r cmd=ping

@jiafu1115
Copy link
Contributor Author

jiafu1115 commented Sep 9, 2016

@christophstrobl hi, is it reasonable to do create this PR? Thanks!
What's more, can not pass the tracivs-ci build due to followed infos.

testWithMessages(org.springframework.data.redis.config.NamespaceTest) Time elapsed: 0.023 sec <<< ERROR!
org.springframework.data.redis.RedisConnectionFailureException: java.net.SocketException: Connection reset; nested exception is redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketException: Connection reset
at org.springframework.data.redis.config.NamespaceTest.testWithMessages(NamespaceTest.java:56)
Caused by: redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketException: Connection reset
at org.springframework.data.redis.config.NamespaceTest.testWithMessages(NamespaceTest.java:56)
Caused by: java.net.SocketException: Connection reset
at org.springframework.data.redis.config.NamespaceTest.testWithMessages(NamespaceTest.java:56)

@christophstrobl
Copy link
Member

Generally speaking pull requests are warmly welcome. Adding tests makes them more likely to be accepted.
Please ensure you've read the contribution guidelines and signed the CLA.

The travis-ci build can be a little shaky from time to time.

@jiafu1115
Copy link
Contributor Author

jiafu1115 commented Sep 9, 2016

@christophstrobl. 3ks for your quick reply. I had added function tests and pass all tests. What's more, I create one issue on JIRA:
https://jira.spring.io/browse/DATAREDIS-552

Thanks for your review!

Motivation: JedisConnectionFactory should support setting clientName into Jedis so that we can trouble shooting easier by client name. now no client name be showed for client list: id=2 addr=10.224.38.23:33652 fd=5 name= age=1350882 idle=1 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 obl=0 oll=0 omem=0 events=r cmd=ping Modifications: add private member: clientName and use it in constructor for Jedis Pool. Result: easier to trouble shooting with connection name: id=2 addr=10.224.38.23:33652 fd=5 name=AppNameOrClientName age=1350882 idle=1 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 obl=0 oll=0 omem=0 events=r cmd=ping
@jiafu1115 jiafu1115 changed the title Support setting client name for connection DATAREDIS-552-Support setting client name for connection with redis server Sep 13, 2016
@mp911de
Copy link
Member

mp911de commented Nov 30, 2016

@jiafu1115 I'll take the PR from here.

@mp911de mp911de self-assigned this Nov 30, 2016
@jiafu1115
Copy link
Contributor Author

@mp911de OK, Thanks, BTW, I know you are the active member for jedis project.

mp911de pushed a commit that referenced this pull request Dec 1, 2016
…Factory. Motivation: JedisConnectionFactory should support setting clientName for Jedis connections so to improve troubleshooting. Result: Easier to troubleshooting with connection name. Original pull request: #219.
mp911de added a commit that referenced this pull request Dec 1, 2016
Apply client name to Sentinel and unpooled connections if set. Add ticket references to JavaDoc. Add tests. Fix existing JavaDoc letter casing and punctuation. Rearrange isClusterAware/isRedisSentinelAware methods. Original pull request: #219.
mp911de pushed a commit that referenced this pull request Dec 1, 2016
…Factory. Motivation: JedisConnectionFactory should support setting clientName for Jedis connections so to improve troubleshooting. Result: Easier to troubleshooting with connection name. Original pull request: #219.
mp911de added a commit that referenced this pull request Dec 1, 2016
Apply client name to Sentinel and unpooled connections if set. Add ticket references to JavaDoc. Add tests. Fix existing JavaDoc letter casing and punctuation. Rearrange isClusterAware/isRedisSentinelAware methods. Original pull request: #219.
@mp911de
Copy link
Member

mp911de commented Dec 1, 2016

Thanks for your PR. That's merged.

@mp911de mp911de closed this Dec 1, 2016
@jiafu1115
Copy link
Contributor Author

@mp911de Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants