Fix NPE while loading lookups from empty JDBC source

[Akshat-Jain]

Description

Currently, when we add a lookup with the lookupExtractorFactory's type as cachedNamespace configured with a JDBC source, we run into NPE while loading the lookup if the configured JDBC source table has no rows.

Sample NPE logs:

2024-04-17T09:44:32,936 ERROR [NamespaceExtractionCacheManager-0] org.apache.druid.server.lookup.namespace.cache.CacheScheduler - Failed to update namespace [JdbcExtractionNamespace{connectorConfig=DbConnectorConfig{createTables=true, connectURI='jdbc:postgresql://localhost:5432/druid', user='druid', passwordProvider=org.apache.druid.metadata.DefaultPasswordProvider, dbcpProperties=null}, table='pglookuptable1', keyColumn='country_id', valueColumn='country_name', tsColumn='timeColumn', filter='null', pollPeriod=PT1M, jitterSeconds=0, loadTimeoutSeconds=120, maxHeapPercentage=10}] : org.apache.druid.server.lookup.namespace.cache.CacheScheduler$EntryImpl@64c4522
java.lang.NullPointerException: null
	at org.apache.druid.server.lookup.namespace.JdbcCacheGenerator.lastUpdates(JdbcCacheGenerator.java:219) ~[?:?]
	at org.apache.druid.server.lookup.namespace.JdbcCacheGenerator.generateCache(JdbcCacheGenerator.java:72) ~[?:?]
	at org.apache.druid.server.lookup.namespace.JdbcCacheGenerator.generateCache(JdbcCacheGenerator.java:48) ~[?:?]
	at org.apache.druid.server.lookup.namespace.cache.CacheScheduler$EntryImpl.tryUpdateCache(CacheScheduler.java:236) ~[?:?]
	at org.apache.druid.server.lookup.namespace.cache.CacheScheduler$EntryImpl.updateCache(CacheScheduler.java:208) ~[?:?]
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
	at java.base/java.util.concurrent.FutureTask.runAndReset$$$capture(FutureTask.java:305) [?:?]
	at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java) [?:?]
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) [?:?]
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
	at java.base/java.lang.Thread.run(Thread.java:829) [?:?]

This PR handles the NPE, and adds a test for validating the corresponding flow.

This PR has:

• been self-reviewed.
• added documentation for new or modified features or behaviors.
• a release note entry in the PR description.
• added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• added or updated version, license, or notice information in licenses.yaml
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
• added integration tests.
• been tested in a test Druid cluster.

[cryptoe]

Changes lgtm.
Thanks for your first pr in apache druid.
🚀

[cryptoe]

Lets add the look up name here.

[Akshat-Jain]

@cryptoe The cache scheduler layer doesn't seem to have lookup name information. It only has the JdbcExtractionNamespace level information - which is what gets logged everywhere else in the cache scheduler layer as well.

Technically we can add an extra method param to send lookup name info all the way down to the cache layer through the whole method chain, but that seems unnecessary to me. That would also then require us to change logging everywhere else in the cache layer, just to ensure consistency - so it would end up as a much wider change.

Thoughts?

[kfaraz]

You can do this:

Suggested change

	LOG.info("Lookup table is empty. No rows returned for the query: %s", query);
	LOG.info("Lookup table[%s] is empty. No rows returned for the query[%s].", table, query);

[Akshat-Jain]

@kfaraz Have made the suggested change.

[LakshSingla]

Wouldn't INFO logging be too noisy, given that this will log every time we run a check (to see whether to update the table or not)? (By default, it is set to not poll, so the default behaviour is cool)

[Akshat-Jain]

@LakshSingla This is going to be logged only if the table is empty, so I think it should be fine.

[kfaraz]

Thanks for the fix, @Akshat-Jain !