clintropolis commented on code in PR #15103:
URL: https://github.com/apache/druid/pull/15103#discussion_r1349352093
##########
docs/development/extensions-core/lookups-cached-global.md:
##########
@@ -352,6 +352,7 @@ The JDBC lookups will poll a database to populate its local
cache. If the `tsCol
|`filter`|The filter to use when selecting lookups, this is used to create a
where clause on lookup population|No|No Filter|
|`tsColumn`| The column in `table` which contains when the key was
updated|No|Not used|
|`pollPeriod`|How often to poll the DB|No|0 (only once)|
+|`jitterSeconds`| How much jitter to add (in seconds) as a delay, used to
distribute db load more evenly|No|0
Review Comment:
table formatting missing ending `|`
```suggestion
|`jitterSeconds`| How much jitter to add (in seconds) as a delay, used to
distribute db load more evenly|No|0|
```
##########
extensions-core/lookups-cached-global/src/main/java/org/apache/druid/query/lookup/namespace/ExtractionNamespace.java:
##########
@@ -41,4 +41,12 @@ default long getMaxHeapPercentage()
{
return -1L;
}
+
+ // For larger clusters, when they all startup at the same time and have
lookups in the db,
+ // it overwhelms the database, this allows implementations to introduce a
jitter, which
+ // should spread out the load.
+ default long getJitter()
Review Comment:
should this be `getJitterMillis()`?
##########
docs/development/extensions-core/lookups-cached-global.md:
##########
@@ -352,6 +352,7 @@ The JDBC lookups will poll a database to populate its local
cache. If the `tsCol
|`filter`|The filter to use when selecting lookups, this is used to create a
where clause on lookup population|No|No Filter|
|`tsColumn`| The column in `table` which contains when the key was
updated|No|Not used|
|`pollPeriod`|How often to poll the DB|No|0 (only once)|
+|`jitterSeconds`| How much jitter to add (in seconds) as a delay, used to
distribute db load more evenly|No|0
Review Comment:
also should we indicate that the actual value used will be random up to this
maximum?
##########
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java:
##########
@@ -63,6 +63,9 @@
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assert.assertTrue;
Review Comment:
nit: we usually try to avoid static imports, like I see no instances of
`import static junit.framework.TestCase.assertEquals` (also, i think we usually
use `Assert.assertEquals`) and only 28 for `import static
org.junit.Assert.assertTrue` across the codebase
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
