Reputation: 335
I have the following logic for reading and writing redis state
export async function updateRedis() {
let stateName = 'stateName'
try {
let isSuccess = false
while (!isSuccess) {
try {
await redis
.watch(stateName, function (err) {
if (err) {
logger.error(`Error in watch state: ${err}`)
}
redis.get(stateName, function (err, result) {
if (err) {
logger.error(`Error in get state: ${err}`)
}
let state = JSON.parse(result)
// do some processing
redis.multi()
.set(stateName, JSON.stringify(state))
.exec(function (err, result) {
if (err) {
logger.error(`Error in set state: ${err}`)
}
if (result != null) {
isSuccess = true
}
})
console.log(`isSuccess for ${stateName} `, isSuccess)
})
})
} catch (e) {
logger.error(`Error: ${e}`)
}
}
} catch (e) {
logger.error(`Error: ${e}`)
}
return Promise.resolve(true)
}
This will print out
"isSuccess for stateName false"
"isSuccess for stateName true"
"isSuccess for stateName true"
So after the flag changes to true, it will continue for more loops. Sometimes it does more than just once.
Am I doing something wrong?
Upvotes: 1
Views: 130
Reputation: 338238
You cannot mix a synchronous loop (while (!isSuccess) { ... }
) with asynchronous functions (redis.watch(stateName, function (err) { ... })
).
You can also not await
callback-based asynchronous functions. A function must return a promise to be awaitable. Since node-redis gives you promises when you don't pass callbacks to its methods, the key is not to do that (redis.watch(stateName, function (err) { ... })
→ redis.watch(stateName)
).
Your approach needs to be redone.
Let's make a function that encapsulates a redis transaction with optimistic locking. It takes a connection object, a key, and a value-transforming function, and it returns the result of the .set()
operation:
const redisTransaction = async (client, key, transformer) => {
// https://github.com/redis/node-redis/blob/master/docs/isolated-execution.md
return client.executeIsolated(async isolatedClient => {
await isolatedClient.watch(key);
const val = await isolatedClient.get(key);
return isolatedClient.multi()
.set(key, await transformer.call(isolatedClient, val))
.exec();
});
};
Now you can await
this function, because it returns a promise. That means we can make a simple infinite loop that exits immediately in case of success (via return
), or retries indefinitely.
export async function updateRedis(key, transformer) {
while (true) {
try {
return await redisTransaction(redis, key, transformer);
} catch (err) {
logger.error(`Error for state ${key}: ${err}`);
}
}
}
A transformer function takes a value, and returns a new value. Inside it, the this
keyword refers to the isolatedClient
from the transaction, which could be useful if your transformation depends on other values from that client.
const result = await updateRedis('stateName', async function (val) {
const state = JSON.parse(val);
const newState = await modifyStateSomehow(state);
return JSON.stringify(newState);
});
The modifyStateSomehow()
can itself be an asynchronous (i.e. "promise-returning") function. If it's not, you can make the state transformer a regular function by removing the async
and the await
.
Upvotes: 3