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

Introduce elasticsearch.in.bat (i.e. es.in for Windows) #8244

Closed
wants to merge 1 commit into from

Conversation

costin
Copy link
Member

@costin costin commented Oct 27, 2014

Break-out common functionality between elasticsearch.bat and service.bat
Relates #8237

@kimchy
Copy link
Member

kimchy commented Oct 27, 2014

LGTM, can't verify it on windows now, maybe someone else can?

@dadoonet
Copy link
Member

Wondering if we should also change plugin.bat to have a single place where we can define some settings.

For example, if someone want to set path.plugins in elasticsearch.in.bat, it should be taken into account by the plugin.bat command as well.
Though that about memory settings, it does not make sense to start the plugin manager with 4gb RAM is the user set ES_HEAP to 4g!

@gmarz
Copy link
Contributor

gmarz commented Oct 27, 2014

@costin

Just tested this on my machine and found the following issues with service.bat:

(1) If service.bat is ran from outside of the bin directory, then it fails to install:

PS C:\elasticsearch\elasticsearch-1.4.0.Beta1> .\bin\service.bat install
Installing service      :  "elasticsearch-service-x64"
Using JAVA_HOME (64-bit):  "C:\Program Files\Java\jdk1.7.0_71"
'C:\elasticsearch\elasticsearch-1.4.0.Beta1\elasticsearch.in.bat' is not recognized as an internal or external command,
operable program or batch file.
The service 'elasticsearch-service-x64' has been installed.

The cause of this is because %~dp0 resolves to C:\elasticsearch\elasticsearch-1.4.0.Beta1 instead of the script directory as you would expect: C:\elasticsearch\elasticsearch-1.4.0.Beta1\bin. I'm confused as to why this is the case though; it works as expected in elasticsearch.bat.

(2) The service name is installed as Elasticsearch ${project.version} instead of Elasticsearch 1.4.0.Beta.

if NOT "%ES_HEAP_NEWSIZE%" == "" set JAVA_OPTS=%JAVA_OPTS% -Xmn%ES_HEAP_NEWSIZE%

if NOT "%ES_DIRECT_SIZE%" == "" set JAVA_OPTS=%JAVA_OPTS% -XX:MaxDirectMemorySize=%ES_DIRECT_SIZE%
CALL %~dp0elasticsearch.in.bat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@costin The shift command on line 40 is what's throwing off the value of %~dp0. Changing this line to CALL %SCRIPT_DIR%elasticsearch.in.bat fixes the issue. I'm just not sure why this wasn't always a problem?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmarz Thanks. Fixed it through %ES_HOME%\bin (it's what we use through-out the script).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Break-out common functionality between elasticsearch.bat and service.bat
Relates elastic#8237
@s1monw
Copy link
Contributor

s1monw commented Oct 28, 2014

I think this should go into 1.4 as well

@costin
Copy link
Member Author

costin commented Oct 28, 2014

#8243 or some parts of it should go in 1.3 as it provides ES_USE_IPV4 for Windows files. The common elasticsearch.in.bat could be left for 1.4 but the option needs to be added in.

@clintongormley
Copy link

@Mpdreamz could you review this one too please?

@Mpdreamz
Copy link
Member

LGTM 👍 @gmarz's issues seem to have all been resolved.

Passing parameters such as -Des.cluster.name=x still work as well

elasticsearch plugin service seem to work properly no matter from where they are called.

@costin
Copy link
Member Author

costin commented Oct 28, 2014

Rebased as is into 1.4, 1.x and master. Added only ES_USE_IPV4 setting to 1.3 to avoid introducing elasticsearch.in.bat (a potentially large change) to the 1.3.x series.

@costin costin closed this Oct 28, 2014
@costin costin mentioned this pull request Oct 28, 2014
@clintongormley clintongormley changed the title Introduce elasticsearch.in.bat (i.e. es.in for Windows) Packaging: Introduce elasticsearch.in.bat (i.e. es.in for Windows) Nov 3, 2014
@clintongormley clintongormley added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v1.5.0 labels Mar 19, 2015
@clintongormley clintongormley changed the title Packaging: Introduce elasticsearch.in.bat (i.e. es.in for Windows) Introduce elasticsearch.in.bat (i.e. es.in for Windows) Jun 7, 2015
@costin costin deleted the 8237 branch July 6, 2020 16:34
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement Team:Delivery Meta label for Delivery team v1.4.0 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants