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

Consolidate Ivarator Configs #2644

Open
apmoriarty opened this issue Nov 20, 2024 · 4 comments · May be fixed by #2655
Open

Consolidate Ivarator Configs #2644

apmoriarty opened this issue Nov 20, 2024 · 4 comments · May be fixed by #2655
Assignees

Comments

@apmoriarty
Copy link
Collaborator

It would be good to consolidate several ivarator configs together into a single config object. This would improve code readability and reduce the chance that a simple change in one area isn't propagated.

As a first step, consolidate the following section of the ShardQueryLogic into a single IvaratorConfig object. For simplicity we can reuse the object mapper code from the IvaratorCacheDirConfig.

For the sake of backwards compatibility and downstream integration, leave the getter and setter methods for each of these variables, but delegate them to the ivarator config object.

    private List<IvaratorCacheDirConfig> ivaratorCacheDirConfigs = Collections.emptyList();
    private String ivaratorFstHdfsBaseURIs = null;
    private int ivaratorCacheBufferSize = 10000;
    private long ivaratorCacheScanPersistThreshold = 100000L;
    private long ivaratorCacheScanTimeout = 1000L * 60 * 60;
    private int maxFieldIndexRangeSplit = 11;
    private int ivaratorMaxOpenFiles = 100;
    private int ivaratorNumRetries = 2;
    private boolean ivaratorPersistVerify = true;
    private int ivaratorPersistVerifyCount = 100;
    private int maxIvaratorSources = 33;
    private long maxIvaratorSourceWait = 1000L * 60 * 30;
    private long maxIvaratorResults = -1;
@apmoriarty
Copy link
Collaborator Author

Future work could tackle the following

  • Update QueryOptions to use an IvaratorConfig object instead of a bunch of individual query options
  • Update IteratorBuildingVisitor to use an IvaratorConfig object instead of individual variables.
  • Update each of the IvaratorBuilders to use an IvaratorConfig instead of individual variables.

@SethSmucker SethSmucker linked a pull request Nov 25, 2024 that will close this issue
@SethSmucker
Copy link
Collaborator

Should private int maxIvaratorTerms = -1; be included in the IvaratorConfig class as well?

@apmoriarty
Copy link
Collaborator Author

Should private int maxIvaratorTerms = -1; be included in the IvaratorConfig class as well?

No, that variable is only used on the webserver side to enforce a limit on the number of ivarator terms in a query.

@SethSmucker
Copy link
Collaborator

Is the code snippet from ShardQueryLogic or ShardQueryConfiguration? I couldn't find it in the former but it was in the latter. I'm assuming it was SQC but I just wanted to double check.

@lbschanno lbschanno linked a pull request Dec 4, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants