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

No way to specify auto-slice in an AbstractBulkByScrollRequest #53044

Closed
tc027 opened this issue Mar 3, 2020 · 8 comments · Fixed by #53068
Closed

No way to specify auto-slice in an AbstractBulkByScrollRequest #53044

tc027 opened this issue Mar 3, 2020 · 8 comments · Fixed by #53068
Labels
>bug feedback_needed :Search/Search Search-related issues that do not fall into other categories v7.6.2 v7.7.0 v8.0.0-alpha1

Comments

@tc027
Copy link

tc027 commented Mar 3, 2020

Elasticsearch version: 7.6.0

JVM version:
openjdk version "11.0.6" 2020-01-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.6+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.6+10, mixed mode)

OS version: Windows 10

Description of the problem including expected versus actual behavior:
There is no way to specify auto-slice in an AbstractBulkByScrollRequest. AbstractBulkByScrollRequest.setSlices only takes an int but AbstractBaseReindexRestHandler.parseSlices checks for an 'auto' string, which is impossible provide via the Java High Level REST Client. Simply providing '0' instead does not work either as this causes parseSlices to throw an exception.

public static final int AUTO_SLICES = 0;
public static final String AUTO_SLICES_VALUE = "auto";

public Self setSlices(int slices) {
        if (slices < 0) {
            throw new IllegalArgumentException("[slices] must be at least 0 but was [" + slices + "]");
        }
        this.slices = slices;
        return self();
}
private static Integer parseSlices(RestRequest request) {
        String slicesString = request.param("slices");
        if (slicesString == null) {
            return null;
        }

        if (slicesString.equals(AbstractBulkByScrollRequest.AUTO_SLICES_VALUE)) {
            return AbstractBulkByScrollRequest.AUTO_SLICES;
        }

        int slices;
        try {
            slices = Integer.parseInt(slicesString);
        } catch (NumberFormatException e) {
            throw new IllegalArgumentException(
                "[slices] must be a positive integer or the string \"auto\", but was [" + slicesString + "]", e);
        }

        if (slices < 1) {
            throw new IllegalArgumentException(
                "[slices] must be a positive integer or the string \"auto\", but was [" + slicesString + "]");
        }

        return slices;
}

Steps to reproduce:

UpdateByQueryRequest request = new UpdateByQueryRequest(index)
request.setSlices("auto")// does not work due to being string
UpdateByQueryRequest request = new UpdateByQueryRequest(index)
request.setSlices(0)// does not work due to being less than 1
@cbuescher cbuescher added :Core/Features/Java High Level REST Client :Search/Search Search-related issues that do not fall into other categories labels Mar 3, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Java High Level REST Client)

@cbuescher
Copy link
Member

Hi,

thanks for raising the issue. I would like to clarify if all your above observations are based on trying to send the request using the HLRC.

UpdateByQueryRequest request = new UpdateByQueryRequest(index)
request.setSlices(0)// does not work due to being less than 1

I think the problem here is that we provide a way to set the '0' value representing "auto" on the request itself, but we cannot pass this through the Rest layer. I think we need to be more lenient in the above mentioned "parse" method.

cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Mar 3, 2020
Currently the AbstractBulkByScrollRequest accepts slice values of 0 via its
`setSlices` method, denoting the "auto" slicing behaviour that is usable by
settting the "slices=auto" parameter on rest requests. When using the High Level
Rest Client, however, we send the 0 value as an integer, which is then rejected
as invalid by `AbstractBulkByScrollRequest#parseSlices`. Instead of making
parsing of the rest request more lenient, this PR opts for changing the
RequestConverter logic in the client to translate 0 values to "auto" on the rest
requests.

Closes elastic#53044
@cbuescher
Copy link
Member

@tc027 in case my assumption mentioned above is true, I opened a PR that should adress the High Level Rest Client behaviour when translating this parameter.

@tc027
Copy link
Author

tc027 commented Mar 3, 2020

Hi,

thanks for raising the issue. I would like to clarify if all your above observations are based on trying to send the request using the HLRC.

UpdateByQueryRequest request = new UpdateByQueryRequest(index)
request.setSlices(0)// does not work due to being less than 1

I think the problem here is that we provide a way to set the '0' value representing "auto" on the request itself, but we cannot pass this through the Rest layer. I think we need to be more lenient in the above mentioned "parse" method.

That is indeed the case, yes.

@tc027 in case my assumption mentioned above is true, I opened a PR that should adress the High Level Rest Client behaviour when translating this parameter.

Thank you for implementing a fix so promptly.

@jchharris
Copy link

Any chance of getting this into a 7.6.x patch releases?

cbuescher pushed a commit that referenced this issue Mar 6, 2020
Currently the AbstractBulkByScrollRequest accepts slice values of 0 via its
`setSlices` method, denoting the "auto" slicing behaviour that is usable by
settting the "slices=auto" parameter on rest requests. When using the High Level
Rest Client, however, we send the 0 value as an integer, which is then rejected
as invalid by `AbstractBulkByScrollRequest#parseSlices`. Instead of making
parsing of the rest request more lenient, this PR opts for changing the
RequestConverter logic in the client to translate 0 values to "auto" on the rest
requests.

Closes #53044
cbuescher pushed a commit that referenced this issue Mar 6, 2020
Currently the AbstractBulkByScrollRequest accepts slice values of 0 via its
`setSlices` method, denoting the "auto" slicing behaviour that is usable by
settting the "slices=auto" parameter on rest requests. When using the High Level
Rest Client, however, we send the 0 value as an integer, which is then rejected
as invalid by `AbstractBulkByScrollRequest#parseSlices`. Instead of making
parsing of the rest request more lenient, this PR opts for changing the
RequestConverter logic in the client to translate 0 values to "auto" on the rest
requests.

Closes #53044
cbuescher pushed a commit that referenced this issue Mar 6, 2020
Currently the AbstractBulkByScrollRequest accepts slice values of 0 via its
`setSlices` method, denoting the "auto" slicing behaviour that is usable by
settting the "slices=auto" parameter on rest requests. When using the High Level
Rest Client, however, we send the 0 value as an integer, which is then rejected
as invalid by `AbstractBulkByScrollRequest#parseSlices`. Instead of making
parsing of the rest request more lenient, this PR opts for changing the
RequestConverter logic in the client to translate 0 values to "auto" on the rest
requests.

Closes #53044
@cbuescher
Copy link
Member

Any chance of getting this into a 7.6.x patch releases?

Should be in 7.6.2

@Camalot9
Copy link

Camalot9 commented Jun 29, 2022

It would be really nice to have this feature in ReindexRequest as well. The documentation says slices can be set to auto but it's required to be a positive long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug feedback_needed :Search/Search Search-related issues that do not fall into other categories v7.6.2 v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants