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

NPE when querying an expired (TTL) JSON document #131

Closed
Aurh1l opened this issue Nov 10, 2022 · 17 comments
Closed

NPE when querying an expired (TTL) JSON document #131

Aurh1l opened this issue Nov 10, 2022 · 17 comments

Comments

@Aurh1l
Copy link

Aurh1l commented Nov 10, 2022

I think this a bug related to the TimeToLive but I may be wrong, when a query looks for an element of a key, in my case a hash compute with some data, that has been erased by the time to live, there is a null pointer exception thrown:

java.lang.NullPointerException
at com.redis.om.spring.repository.query.RediSearchQuery.executeQuery(RediSearchQuery.java:344)
at com.redis.om.spring.repository.query.RediSearchQuery.execute(RediSearchQuery.java:267)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:137)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:121)
at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java:159)
at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:138)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
at com.sun.proxy.$Proxy174.findByHash(Unknown Source)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.base/java.lang.reflect.Method.invoke(Unknown Source)
at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
at com.sun.proxy.$Proxy174.findByHash(Unknown Source)

Here is the class:

@document(value = "XXXXXX", timeToLive = 10 * 60)
@DaTa
public class XXXXXX {

@id
private String id;

@indexed
@nonnull
private String hash;
}

@bsbodden
Copy link
Contributor

@Paure42 thank you. Can you also post the repository code if any?

@Aurh1l
Copy link
Author

Aurh1l commented Nov 10, 2022

public interface XXXXX extends RedisDocumentRepository<XXXXX, String> {
Optional findByHash(final String hash);
}

@bsbodden
Copy link
Contributor

and one more question, is this with v0.6.2 or the v0.6.3-SNAPSHOT? Thanks!

@Aurh1l
Copy link
Author

Aurh1l commented Nov 10, 2022

Both of them

@bsbodden
Copy link
Contributor

My guess is the TTL expires the key, but the key is still in the index at the time of query, but then it NPEs when trying to retrieve the payload of the object.

@Aurh1l
Copy link
Author

Aurh1l commented Nov 10, 2022

Yes I think too

@bsbodden
Copy link
Contributor

@Paure42 I still can't seem to be able to replicate it. I tried a test like:

  
  private final CountDownLatch waiter = new CountDownLatch(1);
  
  @Test
  void testExpiredEntitiesAreNotFound() throws InterruptedException {
    ExpiringPerson kZuse = ExpiringPerson.of("Konrad Zuse", 1L);
    ExpiringPerson woz = ExpiringPerson.of("Steve Wozniak", 1L);
    withTTLAnnotationRepository.saveAll(List.of(kZuse, woz));
    waiter.await(3, TimeUnit.SECONDS);
    assertThat(withTTLAnnotationRepository.count()).isZero();
    assertThat(withTTLAnnotationRepository.findAll()).isEmpty();
    assertThat(withTTLAnnotationRepository.findOneByName("Konrad Zuse")).isNotPresent();
    assertThat(withTTLAnnotationRepository.findOneByName("Steve Wozniak")).isNotPresent();
  }

But it passes without any code changes. Any ideas that will make it closer to your impl?

Entity model:

@Data
@NoArgsConstructor
@RequiredArgsConstructor(staticName = "of")
@Document(timeToLive = 5)
public class ExpiringPerson {
  @Id String id;
  @NonNull @Indexed
  String name;
  
  @NonNull
  @TimeToLive Long ttl;
}

and repo:

public interface ExpiringPersonRepository extends RedisDocumentRepository<ExpiringPerson, String> {
  Optional<ExpiringPerson> findOneByName(String name);
}

@bsbodden
Copy link
Contributor

FYI, I did add a guard for NPE in the RediSearchQuery class which should fix the issue even if I can reproduce.

@bsbodden bsbodden changed the title Null Pointer Exception when looking for a removed element NPE when querying an expired (TTL) JSON document Nov 10, 2022
bsbodden added a commit to bsbodden/redis-om-spring that referenced this issue Nov 10, 2022
@bsbodden
Copy link
Contributor

@Paure42 if you pull 0.6.3-SNAPSHOT this should be fixed.

@Aurh1l
Copy link
Author

Aurh1l commented Nov 14, 2022

Thank you for your work, but the NPE is still there :

at com.redis.om.spring.repository.query.RediSearchQuery.executeQuery(RediSearchQuery.java:362)
at com.redis.om.spring.repository.query.RediSearchQuery.execute(RediSearchQuery.java:276)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:137)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:121)
at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java:159)
at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:138)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
at com.sun.proxy.$Proxy171.findByHash(Unknown Source)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.base/java.lang.reflect.Method.invoke(Unknown Source)
at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
at com.sun.proxy.$Proxy171.findByHash(Unknown Source)

Your impl is close to mine, I don't know why you can't reproduce it.

@bsbodden
Copy link
Contributor

Could you create a self-contained reproducer project?

@bsbodden
Copy link
Contributor

Also, try to clear your Maven cache since I pushed that SNAPSHOT a few timed

@Aurh1l
Copy link
Author

Aurh1l commented Nov 14, 2022

My maven cache was cleaned up when I tried

I will try to create a self-contained reproducer project

@Aurh1l
Copy link
Author

Aurh1l commented Nov 14, 2022

I think I found something interesting.

Here is my class:

@Document(value = "MODEL", timeToLive = 1)
@Data
public class RedisModelEntity {
	@Id
	private String id;

	@Indexed
	@NonNull
	private String hash;

	@Indexed
	private Long otherId;
}

My repository :

@Document(value = "MODEL", timeToLive = 1)
@Data
public class RedisModelEntity {
	@Id
	private String id;

	@Indexed
	@NonNull
	private String hash;

	@Indexed
	private Long otherId;
}

And my code:

@Component
public class TriggerBug {

	@Autowired private RedisModelRepository repository;

	@Value("AAAAA")
	private String hash1;
	@Value("BBBBB")
	private String hash2;

	private static final Logger logger = LoggerFactory.getLogger(TriggerBug.class);

	public void run() throws InterruptedException {
			repository.save(new RedisModelEntity(hash1));
			repository.save(new RedisModelEntity(hash2));
			Thread.sleep(1000);
			Optional<RedisModelEntity> opt = repository.findByHash(hash2);
			if (opt.isEmpty()) {
				logger.info("Empty");
			} else {
				logger.info("Present");
			}
	}
}

This cause NPE :

Caused by: java.lang.NullPointerException: null
	at com.redis.om.spring.repository.query.RediSearchQuery.executeQuery(RediSearchQuery.java:362) ~[redis-om-spring-0.6.3-20221110.233146-3.jar:na]
	at com.redis.om.spring.repository.query.RediSearchQuery.execute(RediSearchQuery.java:276) ~[redis-om-spring-0.6.3-20221110.233146-3.jar:na]

However if I remove repository.save(new RedisModelEntity(hash2)); this will not cause NPE.
And if I let repository.save(new RedisModelEntity(hash2)); but I update the sleep from 1 to 2 seconds the NPE is not thrown.

Seems to be some case of race condition.

@bsbodden
Copy link
Contributor

There is a non-negligible time between an object being saved and the index getting updated. I added an extra check for null on the root of the JSON document returned by the engine. I just pushed a new version of the SNAPSHOT. Clear your cache again and give it a try please.

bsbodden added a commit to bsbodden/redis-om-spring that referenced this issue Nov 14, 2022
@Aurh1l
Copy link
Author

Aurh1l commented Nov 15, 2022

Works for me !

@bsbodden
Copy link
Contributor

Thanks, @Paure42! Keep them coming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants