Consolidate Ivarator Config #2655
Conversation
SethSmucker
force-pushed
the
task/consolidate-ivaratorconfig
branch
from
ac4ed57 to
dd97c1e
Compare
SethSmucker
force-pushed
the
task/consolidate-ivaratorconfig
branch
from
172aba6 to
94ce5ee
Compare
| @Test | ||
| public void testSerializeDeserializeEquivalence() throws JsonProcessingException { | ||
| IvaratorConfig conf1 = new IvaratorConfig(); | ||
| conf1.setIvaratorFstHdfsBaseURIs("hdfs://example.com/ivarator"); |
There was a problem hiding this comment.
The URI won't have a dot com in it, a reasonable value for tests would be something like "file://tmp/ivarator" or "hdfs://tmp/ivarator/"
| @@ -1507,17 +1495,6 @@ public boolean validateOptions(Map<String,String> options) { | |||
| this.termCounts = getMapSerDe().deserializeFromString(serializedMap); | |||
| } | |||
|
|
|||
| // parse out cardinality threshold | |||
There was a problem hiding this comment.
It looks like cardinality threshold stuff was reverted as part of this work, please correct this
| } | ||
|
|
||
| public long getMaxIvaratorResults() { | ||
| return maxIvaratorResults; | ||
| return ivaratorConfig.getMaxIvaratorResults(); |
There was a problem hiding this comment.
Let's follow a "get or create" pattern. This allows lazy initialization of the ivarator config. This means a query that does not require an ivarator can avoid some extra object initialization (and later, some potentially expensive I/O calls to the configured filesystem).
This would become
getIvaratorConfig().setMaxIvaratorResults(maxIvaratorResults)
..
return getIvaratorConfig().getMaxIvaratorResults()
..
public IvaratorConfig getIvaratorConfig(){
if (ivaratorConfig == null) {
// create a fresh config with default values
ivaratorConfig = new IvaratorConfig();
}
return ivaratorConfig;
}| import datawave.query.iterator.ivarator.IvaratorCacheDirConfig; | ||
| import datawave.query.util.sortedset.FileSortedSet; | ||
|
|
||
| public class IvaratorConfig implements Serializable { |
There was a problem hiding this comment.
This looks great! Add a class level comment that this class encapsulates all ivarator configs in a single object to simplify serialization of configs between the webservice and tablet servers
Merges IvaratorConfigs into one central object to be used in
ShardQueryLogicShardQueryConfiguration. Setters/Getters remain the same.#2644