Skip to content

Revert changes on pipeline connection initialization #154

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

Merged
khorshuheng merged 2 commits into from
Jun 29, 2022
Merged

Revert changes on pipeline connection initialization #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9e1704
Revert "Instantiate Jedis client lazily and only once per JVM process…
khorshuheng Jun 29, 2022
d544c2e
Revert "Retry connection failure while instantiating connection provi…
khorshuheng Jun 29, 2022
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
37 changes: 0 additions & 37 deletions spark/ingestion/src/main/scala/feast/ingestion/errorhanders/RetryStrategy.scala
View file
Open in desktop

This file was deleted.

16 changes: 1 addition & 15 deletions spark/ingestion/src/main/scala/feast/ingestion/stores/redis/ClusterPipelineProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,10 @@
*/
package feast.ingestion.stores.redis

import feast.ingestion.errorhanders.RetryStrategy
import redis.clients.jedis.exceptions.JedisClusterOperationException
import redis.clients.jedis.{ClusterPipeline, DefaultJedisClientConfig, HostAndPort}
import redis.clients.jedis.providers.ClusterConnectionProvider

import scala.collection.JavaConverters._
import scala.concurrent.duration.DurationInt
import scala.util.{Failure, Success, Try}

/**
* Provide pipeline for Redis cluster.
Expand All @@ -35,17 +31,7 @@ case class ClusterPipelineProvider(endpoint: RedisEndpoint) extends PipelineProv
.builder()
.password(endpoint.password)
.build()
val MAX_RECONNECTION_ATTEMPT = 2
val RETRY_INTERVAL = 2.seconds
val provider = RetryStrategy.fixedBackOff(RETRY_INTERVAL, MAX_RECONNECTION_ATTEMPT)(getProvider)

def getProvider: Either[JedisClusterOperationException, ClusterConnectionProvider] = {
Try { new ClusterConnectionProvider(nodes, DEFAULT_CLIENT_CONFIG) } match {
case Success(provider) => Right(provider)
case Failure(e: JedisClusterOperationException) => Left(e)
case Failure(e) => throw e
}
}
val provider = new ClusterConnectionProvider(nodes, DEFAULT_CLIENT_CONFIG)

/**
* @return a cluster pipeline
Expand Down
13 changes: 7 additions & 6 deletions spark/ingestion/src/main/scala/feast/ingestion/stores/redis/RedisSinkRelation.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ class RedisSinkRelation(override val sqlContext: SQLContext, config: SparkRedisC

lazy val isClusterMode: Boolean = checkIfInClusterMode(endpoint)

lazy val pipelineProvider: PipelineProvider = if (isClusterMode) {
ClusterPipelineProvider(endpoint)
} else {
SingleNodePipelineProvider(newJedisClient(endpoint))
}

def newJedisClient(endpoint: RedisEndpoint): Jedis = {
val jedis = new Jedis(endpoint.host, endpoint.port)
if (endpoint.password.nonEmpty) {
Expand Down Expand Up @@ -101,6 +95,12 @@ class RedisSinkRelation(override val sqlContext: SQLContext, config: SparkRedisC
java.security.Security.setProperty("networkaddress.cache.ttl", "3");
java.security.Security.setProperty("networkaddress.cache.negative.ttl", "0");

val pipelineProvider = if (isClusterMode) {
ClusterPipelineProvider(endpoint)
} else {
SingleNodePipelineProvider(newJedisClient(endpoint))
}

// grouped iterator to only allocate memory for a portion of rows
partition.grouped(properties.pipelineSize).foreach { batch =>
// group by key and keep only latest row per each key
Expand Down Expand Up @@ -146,6 +146,7 @@ class RedisSinkRelation(override val sqlContext: SQLContext, config: SparkRedisC
}
writePipeline.close()
}
pipelineProvider.close()
}
dataToStore.unpersist()
}
Expand Down
68 changes: 0 additions & 68 deletions spark/ingestion/src/test/scala/feast/ingestion/errorhandlers/RetryStrategySpec.scala
View file
Open in desktop

This file was deleted.