redis / lettuce

Advanced Java Redis client for thread-safe sync, async, and reactive usage. Supports Cluster, Sentinel, Pipelining, and codecs.
https://lettuce.io
MIT License
5.37k stars 962 forks source link

georaduis command execute error when use ReadFrom.REPLICA_PREFERRED #2832

Closed kennethfan closed 3 months ago

kennethfan commented 5 months ago

Bug Report

Current Behavior

Stack trace ```java org.springframework.data.redis.RedisSystemException: Error in execution; nested exception is io.lettuce.core.RedisReadOnlyException: READONL Y You can't write against a read only replica. at org.springframework.data.redis.connection.lettuce.LettuceExceptionConverter.convert(LettuceExceptionConverter.java:54) at org.springframework.data.redis.connection.lettuce.LettuceExceptionConverter.convert(LettuceExceptionConverter.java:52) at org.springframework.data.redis.connection.lettuce.LettuceExceptionConverter.convert(LettuceExceptionConverter.java:41) at org.springframework.data.redis.PassThroughExceptionTranslationStrategy.translate(PassThroughExceptionTranslationStrategy.java:44) at org.springframework.data.redis.FallbackExceptionTranslationStrategy.translate(FallbackExceptionTranslationStrategy.java:42) at org.springframework.data.redis.connection.lettuce.LettuceConnection.convertLettuceAccessException(LettuceConnection.java:272) at org.springframework.data.redis.connection.lettuce.LettuceConnection.await(LettuceConnection.java:1063) at org.springframework.data.redis.connection.lettuce.LettuceConnection.lambda$doInvoke$4(LettuceConnection.java:920) at org.springframework.data.redis.connection.lettuce.LettuceInvoker$Synchronizer.invoke(LettuceInvoker.java:673) at org.springframework.data.redis.connection.lettuce.LettuceInvoker$DefaultSingleInvocationSpec.get(LettuceInvoker.java:589) at org.springframework.data.redis.connection.lettuce.LettuceGeoCommands.geoRadius(LettuceGeoCommands.java:211) at org.springframework.data.redis.connection.DefaultedRedisConnection.geoRadius(DefaultedRedisConnection.java:1520) at org.springframework.data.redis.core.DefaultGeoOperations.lambda$radius$7(DefaultGeoOperations.java:185) at org.springframework.data.redis.core.RedisTemplate.execute(RedisTemplate.java:223) at org.springframework.data.redis.core.RedisTemplate.execute(RedisTemplate.java:190) at org.springframework.data.redis.core.AbstractOperations.execute(AbstractOperations.java:97) at org.springframework.data.redis.core.DefaultGeoOperations.radius(DefaultGeoOperations.java:185) at org.springframework.data.redis.core.DefaultBoundGeoOperations.radius(DefaultBoundGeoOperations.java:144) at org.ggcc.commons.utils.RedisGeographyUtil.nearbyListByPosition(RedisGeographyUtil.java:64) at org.ggcc.ticket.service.impl.OperatorMiniServiceImpl.getShopInfoListByPosition(OperatorMiniServiceImpl.java:213) at org.ggcc.ticket.service.impl.OperatorMiniServiceImpl.getShopTypeList(OperatorMiniServiceImpl.java:490) at org.ggcc.ticket.controller.OperatorMiniController.sw$original$getShopTypeList$ua8s811(OperatorMiniController.java:83) at org.ggcc.ticket.controller.OperatorMiniController.sw$original$getShopTypeList$ua8s811$accessor$sw$b61kld3(OperatorMiniController.java) at org.ggcc.ticket.controller.OperatorMiniController$sw$auxiliary$rign841.call(Unknown Source) at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:86) ```

Input Code

Input Code ```java @ConditionalOnProperty(value = "spring.redis.replica-preferred", havingValue = "true") @Component @Slf4j public class RedisPreferredLettuceClientConfigurationBuilderCustomizer implements LettuceClientConfigurationBuilderCustomizer, InitializingBean { @Override public void customize(LettuceClientConfiguration.LettuceClientConfigurationBuilder clientConfigurationBuilder) { log.info("RedisPreferredLettuceClientConfigurationBuilderCustomizer.customize"); clientConfigurationBuilder.readFrom(ReadFrom.REPLICA_PREFERRED); } @Override public void afterPropertiesSet() throws Exception { log.info("RedisPreferredLettuceClientConfigurationBuilderCustomizer inited"); } } public List> nearbyListByPosition(String key, double longitude, double latitude, int distance, int limit, Class clazz) { BoundGeoOperations boundGeoOperations = geoRedisTemple.boundGeoOps(key); RedisGeoCommands.GeoRadiusCommandArgs geoRadiusCommandArgs = RedisGeoCommands.GeoRadiusCommandArgs .newGeoRadiusArgs().includeDistance().limit(limit).sortAscending(); List>> geoResultList = Objects.requireNonNull( /** * * the under line caused the bug */ boundGeoOperations.radius(new Circle(new Point(longitude, latitude), distance), geoRadiusCommandArgs) ).getContent(); if (CollectionUtil.isEmpty(geoResultList)) { log.info("缓存中没有推荐数据!"); return Lists.newArrayList(); } return geoResultList.stream().map(geoResult -> { JSONObject jsonObject = JSON.parseObject(JSON.toJSONString(geoResult)); GeographyDTO geography = new GeographyDTO<>(); geography.setObject(JSON .parseObject(JSON.toJSONString(JSON.parseObject(JSON.toJSONString(jsonObject.get("content"))) .get("name")),clazz)); geography.setDistance(geoResult.getDistance().getValue()); return geography; }).collect(Collectors.toList()); } ```

Expected behavior/code

Environment

Possible Solution

Additional context

kennethfan commented 5 months ago

io.lettuce.core.masterreplica.ReadOnlyCommands. READ_ONLY_COMMANDS contains geo like command

image image

kennethfan commented 5 months ago

https://github.com/redis/redis/issues/4084

GEORADIUS cannot be executed on readonly slave node #4084

tishun commented 5 months ago

redis/redis#4084

GEORADIUS cannot be executed on readonly slave node #4084

Oh man, what a mess ...

Would using the GEORADIUS_RO work in your case? Do you have a solution to your problem?

kennethfan commented 5 months ago

I create a new class, and use reflection to modify the field ReadOnlyCommands. READ_ONLY_COMMANDS before my custom LettuceClientConfigurationBuilderCustomizer initialize
it works

however, i think is a bug in Lettuce, so i report it as a bug

kennethfan commented 5 months ago
public class ReadOnlyCommandsFix {
    public static void fixRoCommands() throws NoSuchFieldException, IllegalAccessException {
        Class clazz = ReadOnlyCommands.class;
        Field field = clazz.getDeclaredField("READ_ONLY_COMMANDS");
        field.setAccessible(true);
        Set<CommandType> originReadOnlyCommands = (Set<CommandType>) field.get(null);
        Set<CommandType> geoCommands = originReadOnlyCommands.stream()
                .filter(e -> e.name().startsWith("GEO"))
                .collect(Collectors.toSet());
        originReadOnlyCommands.removeAll(geoCommands);
    }
}

this is my way to resolve it , it is ugly

tishun commented 5 months ago

Can you elaborate more on why you need to remove them from the list in this way? Is some part of your code depending on iterating over the list? I assume you can't make a check there?

I agree that the GEORADIUS and GEORADIUSBYMEMBER commands do not belong to this list and we need to remove them. The other GEO* commands in the list should be read-only so they need not be removed.

kennethfan commented 5 months ago

Beacause I have not fund a way to modify or extend the ReadOnlyCommands.isReadOnlyCommand method So I use a bad way to resolve it I have not test all the geo like commands, so I remove all the geo like commands, it is not exact

kennethfan commented 5 months ago

you are right I agree with you

kennethfan commented 4 months ago

Perhaps Eval should also be removed

thachlp commented 3 months ago

Hi @tishun, @kennethfan I checked and removed some not read-only commands on the list. Please help review 🙇

tishun commented 3 months ago

Beacause I have not fund a way to modify or extend the ReadOnlyCommands.isReadOnlyCommand method So I use a bad way to resolve it I have not test all the geo like commands, so I remove all the geo like commands, it is not exact

There is a way to do that, see https://github.com/redis/lettuce/issues/2568#issuecomment-1854036055

Perhaps Eval should also be removed

This has been fixed since 6.3.x

kennethfan commented 1 month ago

good job