[
https://issues.apache.org/jira/browse/SOLR-18644?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jan Høydahl updated SOLR-18644:
-------------------------------
Description:
*Found by CATS*, the cross-platform CLI test framework proposed in SOLR-18642 —
see [PR #5323|https://github.com/apache/solr/pull/5323]. Unlike the other bugs
that PR turned up, this one is *not* marked {{@AwaitsFix}} there: the harness
cannot run test classes in parallel on Windows without it, because {{solr stop
\-\-all}} walks this very directory. The fix will be split out into its own PR
and land first.
h3. The bug
Three places agree on the variable and its default. {{bin/solr.cmd}} was the
only one not reading it:
|| where || what it does ||
| {{bin/solr}} line 123 | {{: "${SOLR_PID_DIR:=$SOLR_TIP/bin}"}}, then used
throughout |
| {{SolrProcessManager}} line 100 | reads the {{solr.pid.dir}} system property,
which {{bin/solr}} passes from {{SOLR_PID_DIR}} |
| {{bin/solr.cmd}} | hardcodes the {{bin}} directory under {{%SOLR_TIP%}} in
five places |
So on Windows a user who sets {{SOLR_PID_DIR}} is silently ignored: the port
files are written to {{bin}} under {{%SOLR_TIP%}}, and {{stop \-\-all}} and
{{status}} then look there too — self-consistent, but not what was asked for,
and not what {{bin/solr}} does.
h3. It contradicts the documentation
{{solr-control-script-reference.adoc}} says, without qualifying it to POSIX:
{quote}The {{status}} command uses the {{SOLR_PID_DIR}} environment variable to
locate Solr process ID files to find running Solr instances, which defaults to
the {{bin}} directory.{quote}
{{taking-solr-to-production.adoc}} also tells the reader to set it.
h3. Why it matters outside tests
The installation directory is exactly where a packaged or read-only install
does not want writes, which is why the variable exists. A Windows service
pointing {{SOLR_PID_DIR}} at a writable location gets the files under
{{%SOLR_TIP%}} anyway; if that directory is not writable the port file silently
never appears, after which {{stop \-\-all}} finds nothing to stop and the node
has to be killed by hand.
h3. Fix
Add the default beside the other {{SOLR_TIP}} handling, and replace the five
hardcoded paths:
{code}
IF NOT DEFINED SOLR_PID_DIR set "SOLR_PID_DIR=%SOLR_TIP%\bin"
{code}
The sites are the {{stop \-\-all}} listing and its {{del}}, the stop-by-port
{{del}}, and the two places that write the port file on start, foreground and
background. Same variable, same default, so nothing changes for anyone who does
not set it.
Not addressed here: the pid file spelling still differs between the platforms
({{solr-<port>.pid}} on bash, {{solr-<port>.port}} on Windows), which
{{SolrProcessManager.pidFilePattern}} already copes with.
🤖 AI Assisted
was:
*Found by CATS*, the cross-platform CLI test framework proposed in SOLR-18642 —
see [PR #5323|https://github.com/apache/solr/pull/5323]. Unlike the other bugs
that PR turned up, this one is *not* marked {{@AwaitsFix}} there: the harness
cannot run test classes in parallel on Windows without it, because {{solr stop
\-\-all}} walks this very directory. The fix will be split out into its own PR
and land first.
h3. The bug
Three places agree on the variable and its default. {{bin/solr.cmd}} was the
only one not reading it:
|| where || what it does ||
| {{bin/solr}} line 123 | {{: "${SOLR_PID_DIR:=$SOLR_TIP/bin}"}}, then used
throughout |
| {{SolrProcessManager}} line 100 | reads the {{solr.pid.dir}} system property,
which {{bin/solr}} passes from {{SOLR_PID_DIR}} |
| {{bin/solr.cmd}} | hardcodes {{%SOLR_TIP%\\bin}} in five places |
So on Windows a user who sets {{SOLR_PID_DIR}} is silently ignored: the port
files are written to {{%SOLR_TIP%\\bin}}, and {{stop \-\-all}} and {{status}}
then look there too — self-consistent, but not what was asked for, and not what
{{bin/solr}} does.
h3. It contradicts the documentation
{{solr-control-script-reference.adoc}} says, without qualifying it to POSIX:
{quote}The {{status}} command uses the {{SOLR_PID_DIR}} environment variable to
locate Solr process ID files to find running Solr instances, which defaults to
the {{bin}} directory.{quote}
{{taking-solr-to-production.adoc}} also tells the reader to set it.
h3. Why it matters outside tests
The installation directory is exactly where a packaged or read-only install
does not want writes, which is why the variable exists. A Windows service
pointing {{SOLR_PID_DIR}} at a writable location gets the files in
{{%SOLR_TIP%\\bin}} anyway; if that directory is not writable the port file
silently never appears, after which {{stop \-\-all}} finds nothing to stop and
the node has to be killed by hand.
h3. Fix
Add the default beside the other {{SOLR_TIP}} handling, and replace the five
hardcoded paths:
{code:bat}
IF NOT DEFINED SOLR_PID_DIR set "SOLR_PID_DIR=%SOLR_TIP%\bin"
{code}
The sites are the {{stop \-\-all}} listing and its {{del}}, the stop-by-port
{{del}}, and the two places that write the port file on start, foreground and
background. Same variable, same default, so nothing changes for anyone who does
not set it.
Not addressed here: the pid file spelling still differs between the platforms
({{solr-<port>.pid}} on bash, {{solr-<port>.port}} on Windows), which
{{SolrProcessManager.pidFilePattern}} already copes with.
🤖 AI Assisted
> bin/solr.cmd ignores SOLR_PID_DIR, writing pid files into the installation
> directory
> ------------------------------------------------------------------------------------
>
> Key: SOLR-18644
> URL: https://issues.apache.org/jira/browse/SOLR-18644
> Project: Solr
> Issue Type: Bug
> Components: cli
> Reporter: Jan Høydahl
> Priority: Major
> Labels: cats, windows
>
> *Found by CATS*, the cross-platform CLI test framework proposed in SOLR-18642
> — see [PR #5323|https://github.com/apache/solr/pull/5323]. Unlike the other
> bugs that PR turned up, this one is *not* marked {{@AwaitsFix}} there: the
> harness cannot run test classes in parallel on Windows without it, because
> {{solr stop \-\-all}} walks this very directory. The fix will be split out
> into its own PR and land first.
> h3. The bug
> Three places agree on the variable and its default. {{bin/solr.cmd}} was the
> only one not reading it:
> || where || what it does ||
> | {{bin/solr}} line 123 | {{: "${SOLR_PID_DIR:=$SOLR_TIP/bin}"}}, then used
> throughout |
> | {{SolrProcessManager}} line 100 | reads the {{solr.pid.dir}} system
> property, which {{bin/solr}} passes from {{SOLR_PID_DIR}} |
> | {{bin/solr.cmd}} | hardcodes the {{bin}} directory under {{%SOLR_TIP%}} in
> five places |
> So on Windows a user who sets {{SOLR_PID_DIR}} is silently ignored: the port
> files are written to {{bin}} under {{%SOLR_TIP%}}, and {{stop \-\-all}} and
> {{status}} then look there too — self-consistent, but not what was asked for,
> and not what {{bin/solr}} does.
> h3. It contradicts the documentation
> {{solr-control-script-reference.adoc}} says, without qualifying it to POSIX:
> {quote}The {{status}} command uses the {{SOLR_PID_DIR}} environment variable
> to locate Solr process ID files to find running Solr instances, which
> defaults to the {{bin}} directory.{quote}
> {{taking-solr-to-production.adoc}} also tells the reader to set it.
> h3. Why it matters outside tests
> The installation directory is exactly where a packaged or read-only install
> does not want writes, which is why the variable exists. A Windows service
> pointing {{SOLR_PID_DIR}} at a writable location gets the files under
> {{%SOLR_TIP%}} anyway; if that directory is not writable the port file
> silently never appears, after which {{stop \-\-all}} finds nothing to stop
> and the node has to be killed by hand.
> h3. Fix
> Add the default beside the other {{SOLR_TIP}} handling, and replace the five
> hardcoded paths:
> {code}
> IF NOT DEFINED SOLR_PID_DIR set "SOLR_PID_DIR=%SOLR_TIP%\bin"
> {code}
> The sites are the {{stop \-\-all}} listing and its {{del}}, the stop-by-port
> {{del}}, and the two places that write the port file on start, foreground and
> background. Same variable, same default, so nothing changes for anyone who
> does not set it.
> Not addressed here: the pid file spelling still differs between the platforms
> ({{solr-<port>.pid}} on bash, {{solr-<port>.port}} on Windows), which
> {{SolrProcessManager.pidFilePattern}} already copes with.
> 🤖 AI Assisted
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
