jihoonson commented on a change in pull request #8272:
URL: https://github.com/apache/druid/pull/8272#discussion_r433620400
##########
File path:
extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/cache/CacheScheduler.java
##########
@@ -381,17 +412,59 @@ public String toString()
ENTRY_CLOSED
}
- public final class VersionedCache implements CacheState, AutoCloseable
+ /**
+ * This builder class holds a {@link VersionedCache} object,
+ * For handling failures while loading cache, it exposes close() method
which will close {@link CacheHandler}
+ */
+ public final class VersionedCacheBuilder
+ {
+
+ public VersionedCache getVersionedCache()
+ {
+ return versionedCache;
+ }
+
+ private final VersionedCache versionedCache;
+
+ public VersionedCacheBuilder(
+ @Nullable EntryImpl<? extends ExtractionNamespace> entryId,
+ String version,
+ @Nullable CacheHandler cacheHandler
+ )
+ {
+ updatesStarted.incrementAndGet();
Review comment:
This gets double-incremented here and at [Line
555](https://github.com/apache/druid/pull/8272/files#diff-f2359bc636b6f71d70297829eb79dc01R555).
##########
File path:
extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/cache/CacheScheduler.java
##########
@@ -381,17 +412,59 @@ public String toString()
ENTRY_CLOSED
}
- public final class VersionedCache implements CacheState, AutoCloseable
+ /**
+ * This builder class holds a {@link VersionedCache} object,
+ * For handling failures while loading cache, it exposes close() method
which will close {@link CacheHandler}
+ */
+ public final class VersionedCacheBuilder
Review comment:
This seems more like a holder rather than a builder. Suggest renaming to
`VersionedCacheHolder`.
##########
File path: docs/development/extensions-core/lookups-cached-global.md
##########
@@ -337,7 +337,7 @@ The `simpleJson` lookupParseSpec does not take any
parameters. It is simply a li
### JDBC lookup
-The JDBC lookups will poll a database to populate its local cache. If the
`tsColumn` is set it must be able to accept comparisons in the format
`'2015-01-01 00:00:00'`. For example, the following must be valid SQL for the
table `SELECT * FROM some_lookup_table WHERE timestamp_column > '2015-01-01
00:00:00'`. If `tsColumn` is set, the caching service will attempt to only poll
values that were written *after* the last sync. If `tsColumn` is not set, the
entire table is pulled every time.
+The JDBC lookups will poll a database to populate its local cache. If the
`tsColumn` is set it must be able to accept comparisons in the format
`'2015-01-01 00:00:00'`. For example, the following must be valid sql for the
table `SELECT * FROM some_lookup_table WHERE timestamp_column > '2015-01-01
00:00:00'`. **If `tsColumn` is set, the caching service will attempt to only
poll values that were written after the last sync, and any entries deleted from
the the database won't be removed from the cache.** If `tsColumn` is not set,
the entire table is pulled every time. Due to possibilities of use-after-free
race conditions, **setting `tsColumn` is not recommended for `offHeap`
lookups.**
Review comment:
Please add more details about why `tsColumn` is not recommended so that
users can be aware of what would happen if they use.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
