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.

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

Modify RedisConfig and use JedisPool #756

Merged
merged 1 commit into from Feb 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions misk-redis/src/main/kotlin/misk/redis/FakeRedis.kt
Expand Up @@ -6,6 +6,7 @@ import java.time.Instant
import java.util.concurrent.ConcurrentHashMap
import javax.inject.Inject

/** Mimics a Redis instance for testing. */
class FakeRedis : Redis {
@Inject lateinit var clock: FakeClock

Expand Down
27 changes: 19 additions & 8 deletions misk-redis/src/main/kotlin/misk/redis/RealRedis.kt
@@ -1,31 +1,42 @@
package misk.redis

import redis.clients.jedis.Jedis
import redis.clients.jedis.JedisPool
import java.time.Duration

class RealRedis(private val jedis : Jedis) : Redis {
/** For each command, a Jedis instance is retrieved from the pool and returned once the command has been issued. */
class RealRedis(private val jedisPool: JedisPool) : Redis {
override fun del(key: String): Boolean {
return (jedis.del(key) == 1L)
jedisPool.resource.use { jedis ->
return (jedis.del(key) == 1L)
}
}

override fun del(vararg keys: String): Int {
return jedis.del(*keys).toInt()
jedisPool.resource.use { jedis ->
return jedis.del(*keys).toInt()
}
}

override fun get(key: String): String? {
return jedis.get(key)
jedisPool.resource.use { jedis ->
return jedis.get(key)
}
}

override fun set(key: String, value: String): String {
return jedis.set(key, value)
jedisPool.resource.use { jedis ->
return jedis.set(key, value)
}
}

override fun set(key: String, expiryDuration: Duration, value: String): String {
return jedis.setex(key, expiryDuration.seconds.toInt(), value)
jedisPool.resource.use { jedis ->
return jedis.setex(key, expiryDuration.seconds.toInt(), value)
}
}

/** Closes the connection to Redis. */
fun close() {
return jedis.close()
return jedisPool.close()
}
}
20 changes: 15 additions & 5 deletions misk-redis/src/main/kotlin/misk/redis/RedisConfig.kt
Expand Up @@ -2,8 +2,18 @@ package misk.redis

import misk.config.Config

data class RedisConfig(
val host_name: String,
val port: Int,
val auth_password: String
) : Config
class RedisConfig: java.util.LinkedHashMap<String, RedisReplicationGroupConfig>, Config {
constructor(): super()
constructor(m: Map<String, RedisReplicationGroupConfig>): super(m)
}

data class RedisReplicationGroupConfig(
val writer_endpoint: RedisNodeConfig,
val reader_endpoint: RedisNodeConfig,
val redis_auth_password: String
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be a Secret. while it's not strictly necessary now because it's a single value, I would assume in the future Secret would allow us to redact the value instead of fuzzy matching on fields named password

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think a Secret makes sense here, I'll keep it in mind

)

data class RedisNodeConfig(
val hostname: String,
val port: Int
)
31 changes: 25 additions & 6 deletions misk-redis/src/main/kotlin/misk/redis/RedisModule.kt
Expand Up @@ -4,18 +4,37 @@ import com.google.common.util.concurrent.Service
import com.google.inject.Provides
import com.google.inject.Singleton
import misk.inject.KAbstractModule
import redis.clients.jedis.Jedis
import redis.clients.jedis.JedisPool
import redis.clients.jedis.JedisPoolConfig

class RedisModule : KAbstractModule() {
/**
* Configures a JedisPool to connect to a Redis instance. The use of a JedisPool ensures thread safety.
* See: https://github.com/xetorthio/jedis/wiki/Getting-started#using-jedis-in-a-multithreaded-environment
*/
class RedisModule(private val jedisPoolConfig: JedisPoolConfig) : KAbstractModule() {
override fun configure() {
multibind<Service>().to<RedisService>()
}

@Provides @Singleton
internal fun provideRedisClient(config: RedisConfig): Redis {
// Connect to the redis instance
val jedis = Jedis(config.host_name, config.port, true)
jedis.auth(config.auth_password)
return RealRedis(jedis)
// Get the first replication group, we only support 1 replication group per service
val replicationGroup = config[config.keys.first()]
?: throw RuntimeException("At least 1 replication group must be specified")

// Create our jedis pool
val jedisPool = JedisPool(
jedisPoolConfig,
replicationGroup.writer_endpoint.hostname,
replicationGroup.writer_endpoint.port,
true
)

// Authenticate with the redis server
jedisPool.resource.use {
it.auth(replicationGroup.redis_auth_password)
}

return RealRedis(jedisPool)
}
}
4 changes: 1 addition & 3 deletions misk-redis/src/main/kotlin/misk/redis/RedisService.kt
Expand Up @@ -5,9 +5,7 @@ import javax.inject.Inject
import javax.inject.Provider
import javax.inject.Singleton

/**
* Controls the connection lifecycle for Redis.
*/
/** Controls the connection lifecycle for Redis. */
@Singleton
internal class RedisService @Inject internal constructor(
private val redisProvider: Provider<Redis>
Expand Down
Expand Up @@ -2,7 +2,7 @@ package misk.redis

import misk.inject.KAbstractModule

internal class RedisTestModule : KAbstractModule() {
class RedisTestModule : KAbstractModule() {
override fun configure() {
bind<Redis>().toInstance(FakeRedis())
}
Expand Down