You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by GitBox <gi...@apache.org> on 2021/03/12 09:32:14 UTC
[GitHub] [bahir-flink] YikSanChan opened a new pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
YikSanChan opened a new pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe merged pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe merged pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-797368208
@YikSanChan its the same commit? if yes could we maintain the original committer?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on a change in pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on a change in pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#discussion_r594191993
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/config/FlinkJedisConfigBase.java
##########
@@ -91,6 +99,18 @@ public int getMinIdle() {
return minIdle;
}
+ public boolean isTestOnBorrow() {
Review comment:
we have the getTestOnBorrow could we use that and eliminate this
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-801738178
Hi @YikSanChan , sorry for the delay..
Any test added yes always welcome
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799277525
@eskabetxe Fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-801768176
@eskabetxe Added tests in flink-connector-redis/src/test/java/org/apache/flink/streaming/connectors/redis/common/container/RedisCommandsContainerBuilderTest.java, let me know if you have any other concern, thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-801528471
@eskabetxe Hi Joao could you please see my questions above? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-802115345
@eskabetxe BTW I wonder what is the release schedule? Can we release a new version of flink-connector-redis after merging this in?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799293656
@eskabetxe Regarding tests, I am thinking of adding a unit test for `RedisCommandsContainerBuilder`, and mainly test the `getGenericObjectPoolConfig` method. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799194451
Hi @YikSanChan, I would like to keep the original committer where possible.
If you left its branch, modify the code with my comments and made a PR, you should be able to keep original committer
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799261823
@YikSanChan I fix the original committer on this PR
could you check that I dont delete anything wrong or something get merged bad
after that I will check the PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe edited a comment on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe edited a comment on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799267033
@YikSanChan could you address those changes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799267033
@YikSanChan could you address those changes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799217175
@eskabetxe Can you please walk me through how am I able to continue with the PR https://github.com/apache/bahir-flink/pull/104?
> If you left its branch
What does "left its branch" mean? This repo only has one branch `master` (https://github.com/apache/bahir-flink/branches), and lmagic233's branch is not in the current repo. I am not able to fetch his commits
```
> git remote add lmagic233 git@github.com:lmagic233/bahir-flink.git
> git remote -v
lmagic233 git@github.com:lmagic233/bahir-flink.git (fetch)
lmagic233 git@github.com:lmagic233/bahir-flink.git (push)
origin https://github.com/YikSanChan/bahir-flink.git (fetch)
origin https://github.com/YikSanChan/bahir-flink.git (push)
upstream https://github.com/apache/bahir-flink.git (fetch)
upstream https://github.com/apache/bahir-flink.git (push)
> git fetch lmagic233
The authenticity of host 'github.com (52.74.223.119)' can't be established.
RSA key fingerprint is SHA256:nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8.
Are you sure you want to continue connecting (yes/no/[fingerprint])? yes
Warning: Permanently added 'github.com,52.74.223.119' (RSA) to the list of known hosts.
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-798424195
@eskabetxe Hi Joao, could you please suggest next steps? 🙏
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on a change in pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on a change in pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#discussion_r594172661
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/config/FlinkJedisClusterConfig.java
##########
@@ -179,25 +185,68 @@ public Builder setPassword(String password) {
return this;
}
+ /**
+ * Sets value for the {@code testOnBorrow} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnBorrow Whether objects borrowed from the pool will be validated before being returned
+ * @return Builder itself
+ */
+ public Builder setTestOnBorrow(boolean testOnBorrow) {
+ this.testOnBorrow = testOnBorrow;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testOnReturn} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnReturn Whether objects borrowed from the pool will be validated when they are returned to the pool
+ * @return Builder itself
+ */
+ public Builder setTestOnReturn(boolean testOnReturn) {
+ this.testOnReturn = testOnReturn;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testWhileIdle} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * Setting this to true will also set default idle-testing parameters provided in Jedis
+ * @see redis.clients.jedis.JedisPoolConfig
+ *
+ * @param testWhileIdle Whether objects sitting idle in the pool will be validated by the idle object evictor
+ * @return Builder itself
+ */
+ public Builder setTestWhileIdle(boolean testWhileIdle) {
+ this.testWhileIdle = testWhileIdle;
+ return this;
+ }
+
/**
* Builds JedisClusterConfig.
*
* @return JedisClusterConfig
*/
public FlinkJedisClusterConfig build() {
- return new FlinkJedisClusterConfig(nodes, timeout, maxRedirections, maxTotal, maxIdle, minIdle, password);
+ return new FlinkJedisClusterConfig(nodes, timeout, maxRedirections, maxTotal, maxIdle, minIdle, password, testOnBorrow, testOnReturn, testWhileIdle);
}
}
@Override
public String toString() {
return "FlinkJedisClusterConfig{" +
- "nodes=" + nodes +
- ", timeout=" + connectionTimeout +
- ", maxRedirections=" + maxRedirections +
- ", maxTotal=" + maxTotal +
- ", maxIdle=" + maxIdle +
- ", minIdle=" + minIdle +
- '}';
+ "nodes=" + nodes +
+ ", maxRedirections=" + maxRedirections +
+ ", maxTotal=" + maxTotal +
+ ", maxIdle=" + maxIdle +
+ ", minIdle=" + minIdle +
+ ", connectionTimeout=" + connectionTimeout +
+ ", password='" + password + '\'' +
Review comment:
remove `'\''`
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/config/FlinkJedisPoolConfig.java
##########
@@ -188,27 +196,69 @@ public Builder setPassword(String password) {
return this;
}
+ /**
+ * Sets value for the {@code testOnBorrow} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnBorrow Whether objects borrowed from the pool will be validated before being returned
+ * @return Builder itself
+ */
+ public Builder setTestOnBorrow(boolean testOnBorrow) {
+ this.testOnBorrow = testOnBorrow;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testOnReturn} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnReturn Whether objects borrowed from the pool will be validated when they are returned to the pool
+ * @return Builder itself
+ */
+ public Builder setTestOnReturn(boolean testOnReturn) {
+ this.testOnReturn = testOnReturn;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testWhileIdle} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * Setting this to true will also set default idle-testing parameters provided in Jedis
+ * @see redis.clients.jedis.JedisPoolConfig
+ *
+ * @param testWhileIdle Whether objects sitting idle in the pool will be validated by the idle object evictor
+ * @return Builder itself
+ */
+ public Builder setTestWhileIdle(boolean testWhileIdle) {
+ this.testWhileIdle = testWhileIdle;
+ return this;
+ }
/**
* Builds JedisPoolConfig.
*
* @return JedisPoolConfig
*/
public FlinkJedisPoolConfig build() {
- return new FlinkJedisPoolConfig(host, port, timeout, password, database, maxTotal, maxIdle, minIdle);
+ return new FlinkJedisPoolConfig(host, port, timeout, password, database, maxTotal, maxIdle, minIdle, testOnBorrow, testOnReturn, testWhileIdle);
}
}
@Override
public String toString() {
return "FlinkJedisPoolConfig{" +
- "host='" + host + '\'' +
- ", port=" + port +
- ", timeout=" + connectionTimeout +
- ", database=" + database +
- ", maxTotal=" + maxTotal +
- ", maxIdle=" + maxIdle +
- ", minIdle=" + minIdle +
- '}';
+ "host='" + host + '\'' +
+ ", port=" + port +
+ ", database=" + database +
+ ", maxTotal=" + maxTotal +
+ ", maxIdle=" + maxIdle +
+ ", minIdle=" + minIdle +
+ ", connectionTimeout=" + connectionTimeout +
+ ", password='" + password + '\'' +
Review comment:
ditto
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/config/FlinkJedisSentinelConfig.java
##########
@@ -223,27 +230,71 @@ public Builder setMinIdle(int minIdle) {
return this;
}
+ /**
+ * Sets value for the {@code testOnBorrow} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnBorrow Whether objects borrowed from the pool will be validated before being returned
+ * @return Builder itself
+ */
+ public Builder setTestOnBorrow(boolean testOnBorrow) {
+ this.testOnBorrow = testOnBorrow;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testOnReturn} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnReturn Whether objects borrowed from the pool will be validated when they are returned to the pool
+ * @return Builder itself
+ */
+ public Builder setTestOnReturn(boolean testOnReturn) {
+ this.testOnReturn = testOnReturn;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testWhileIdle} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * Setting this to true will also set default idle-testing parameters provided in Jedis
+ * @see redis.clients.jedis.JedisPoolConfig
+ *
+ * @param testWhileIdle Whether objects sitting idle in the pool will be validated by the idle object evictor
+ * @return Builder itself
+ */
+ public Builder setTestWhileIdle(boolean testWhileIdle) {
+ this.testWhileIdle = testWhileIdle;
+ return this;
+ }
+
/**
* Builds JedisSentinelConfig.
*
* @return JedisSentinelConfig
*/
public FlinkJedisSentinelConfig build(){
return new FlinkJedisSentinelConfig(masterName, sentinels, connectionTimeout, soTimeout,
- password, database, maxTotal, maxIdle, minIdle);
+ password, database, maxTotal, maxIdle, minIdle, testOnBorrow, testOnReturn, testWhileIdle);
}
}
@Override
public String toString() {
return "FlinkJedisSentinelConfig{" +
- "masterName='" + masterName + '\'' +
- ", connectionTimeout=" + connectionTimeout +
- ", soTimeout=" + soTimeout +
- ", database=" + database +
- ", maxTotal=" + maxTotal +
- ", maxIdle=" + maxIdle +
- ", minIdle=" + minIdle +
- '}';
+ "masterName='" + masterName + '\'' +
Review comment:
ditto
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/config/FlinkJedisSentinelConfig.java
##########
@@ -223,27 +230,71 @@ public Builder setMinIdle(int minIdle) {
return this;
}
+ /**
+ * Sets value for the {@code testOnBorrow} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnBorrow Whether objects borrowed from the pool will be validated before being returned
+ * @return Builder itself
+ */
+ public Builder setTestOnBorrow(boolean testOnBorrow) {
+ this.testOnBorrow = testOnBorrow;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testOnReturn} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * @param testOnReturn Whether objects borrowed from the pool will be validated when they are returned to the pool
+ * @return Builder itself
+ */
+ public Builder setTestOnReturn(boolean testOnReturn) {
+ this.testOnReturn = testOnReturn;
+ return this;
+ }
+
+ /**
+ * Sets value for the {@code testWhileIdle} configuration attribute
+ * for pools to be created with this configuration instance.
+ *
+ * Setting this to true will also set default idle-testing parameters provided in Jedis
+ * @see redis.clients.jedis.JedisPoolConfig
+ *
+ * @param testWhileIdle Whether objects sitting idle in the pool will be validated by the idle object evictor
+ * @return Builder itself
+ */
+ public Builder setTestWhileIdle(boolean testWhileIdle) {
+ this.testWhileIdle = testWhileIdle;
+ return this;
+ }
+
/**
* Builds JedisSentinelConfig.
*
* @return JedisSentinelConfig
*/
public FlinkJedisSentinelConfig build(){
return new FlinkJedisSentinelConfig(masterName, sentinels, connectionTimeout, soTimeout,
- password, database, maxTotal, maxIdle, minIdle);
+ password, database, maxTotal, maxIdle, minIdle, testOnBorrow, testOnReturn, testWhileIdle);
}
}
@Override
public String toString() {
return "FlinkJedisSentinelConfig{" +
- "masterName='" + masterName + '\'' +
- ", connectionTimeout=" + connectionTimeout +
- ", soTimeout=" + soTimeout +
- ", database=" + database +
- ", maxTotal=" + maxTotal +
- ", maxIdle=" + maxIdle +
- ", minIdle=" + minIdle +
- '}';
+ "masterName='" + masterName + '\'' +
+ ", sentinels=" + sentinels +
+ ", soTimeout=" + soTimeout +
+ ", database=" + database +
+ ", maxTotal=" + maxTotal +
+ ", maxIdle=" + maxIdle +
+ ", minIdle=" + minIdle +
+ ", connectionTimeout=" + connectionTimeout +
+ ", password='" + password + '\'' +
Review comment:
ditto
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/container/RedisCommandsContainerBuilder.java
##########
@@ -64,9 +65,17 @@ public static RedisCommandsContainer build(FlinkJedisPoolConfig jedisPoolConfig)
GenericObjectPoolConfig genericObjectPoolConfig = getGenericObjectPoolConfig(jedisPoolConfig);
+ if (jedisPoolConfig.getTestWhileIdle()) {
+ // default parameters from redis.clients.jedis.JedisPoolConfig
+ genericObjectPoolConfig.setTestWhileIdle(true);
+ genericObjectPoolConfig.setMinEvictableIdleTimeMillis(60000);
+ genericObjectPoolConfig.setTimeBetweenEvictionRunsMillis(30000);
+ genericObjectPoolConfig.setNumTestsPerEvictionRun(-1);
+ }
Review comment:
This is no longer needed since they are covered by `getGenericObjectPoolConfig`
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/container/RedisCommandsContainerBuilder.java
##########
@@ -82,12 +91,20 @@ public static RedisCommandsContainer build(FlinkJedisClusterConfig jedisClusterC
GenericObjectPoolConfig genericObjectPoolConfig = getGenericObjectPoolConfig(jedisClusterConfig);
+ if (jedisClusterConfig.getTestWhileIdle()) {
+ // default parameters from redis.clients.jedis.JedisPoolConfig
+ genericObjectPoolConfig.setTestWhileIdle(true);
+ genericObjectPoolConfig.setMinEvictableIdleTimeMillis(60000);
+ genericObjectPoolConfig.setTimeBetweenEvictionRunsMillis(30000);
+ genericObjectPoolConfig.setNumTestsPerEvictionRun(-1);
+ }
Review comment:
ditto
##########
File path: flink-connector-redis/src/main/java/org/apache/flink/streaming/connectors/redis/common/container/RedisCommandsContainerBuilder.java
##########
@@ -103,18 +120,29 @@ public static RedisCommandsContainer build(FlinkJedisSentinelConfig jedisSentine
GenericObjectPoolConfig genericObjectPoolConfig = getGenericObjectPoolConfig(jedisSentinelConfig);
+ if (jedisSentinelConfig.getTestWhileIdle()) {
+ // default parameters from redis.clients.jedis.JedisPoolConfig
+ genericObjectPoolConfig.setTestWhileIdle(true);
+ genericObjectPoolConfig.setMinEvictableIdleTimeMillis(60000);
+ genericObjectPoolConfig.setTimeBetweenEvictionRunsMillis(30000);
+ genericObjectPoolConfig.setNumTestsPerEvictionRun(-1);
+ }
Review comment:
ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] eskabetxe commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
eskabetxe commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-814832535
@YikSanChan thanks for your contribution
merged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-804057060
@eskabetxe Could you please help with another look? Thank you so much!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan edited a comment on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan edited a comment on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-797372392
@eskabetxe Hey it is very similar to the previous commit but addresses your comments. I tried to add an additional commit to the previous PR but I don't have write access to the user's repo.
Let me know what do you think, thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-799269162
@eskabetxe Work in progress
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bahir-flink] YikSanChan commented on pull request #121: [BAHIR-247] Provide connection validation/idle testing for Flink-Redis Connector
Posted by GitBox <gi...@apache.org>.
YikSanChan commented on pull request #121:
URL: https://github.com/apache/bahir-flink/pull/121#issuecomment-797372392
@eskabetxe Hey it is very similar to the previous commit but address your comments. I tried to add an additional commit to the previous PR but I don't have write access to the user's repo.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org