https://pinot.apache.org/ logo
#pinot-dev
Title
# pinot-dev
k

Ken Krugler

09/22/2021, 12:03 AM
We noticed that our automated running of the batch ingestion job (via
bin/pinot-admin.sh
) wasn’t stopping when batch ingest failed. In looking at
PinotAdministrator
, it seems like you have to set the
pinot.admin.system.exit
System property to
true
for this to work. Any reason why
pinot-admin.sh
shouldn’t be setting this to true if
JAVA_OPTS
isn’t specified? E.g. something like
Copy code
if [ -z "$JAVA_OPTS" ] ; then
  ALL_JAVA_OPTS="-Xms4G -Dlog4j2.configurationFile=conf/log4j2.xml -Dpinot.admin.system.exit=true"
else
  ALL_JAVA_OPTS=$JAVA_OPTS
fi
And for some reason, out of all the sub-commands, only
LaunchDataIngestionJobCommand
has its own
main()
method. Any reason why it needs a main method?
k

Kishore G

09/22/2021, 12:14 AM
probably used in some test cases and we dont want them to exit
thats just my guess
k

Ken Krugler

09/22/2021, 12:15 AM
Yes re not always calling System.exit(), but wondering why the bash script doesn’t set up for it to return a status code that way…since I don’t think the test cases are using
pinot-admin.sh
, right?
k

Kishore G

09/22/2021, 12:21 AM
yeah, not sure why.
m

Mayank

09/22/2021, 2:11 AM
Right I remember the same, it was for avoiding test cases calling System.exit(). Don’t think there was a reason for not doing so in the shell script
x

Xiang Fu

09/24/2021, 5:59 PM
I think that may fail the quickstart? say the ingesiton job finished will exit the process
k

Ken Krugler

09/24/2021, 9:28 PM
@User - With my PR, it’s only the pinot-admin.sh script that sets
pinot.admin.system.exit=true
(no change to the default setting in the PinotAdministrator Java code). Do you think that might still cause a problem? Asking because I did see quickstart test failed on the build machine. But I tried
quick-start-batch.sh
on my Mac using a build with my change, and that worked.
x

Xiang Fu

09/24/2021, 10:01 PM
hmmm, pinot quickstart CI starts zk/controller/broker/server/minion separately
can you check that logic
from log, seems zk is not up
k

Ken Krugler

09/24/2021, 10:17 PM
OK, I see what’s going on - calling System.exit(0) terminates the ZK thread that the
bin/pinot-admin.sh StartZookeeper
command used by the CI quickstart spun up, versus the regular return from PinotAdministrator will hang due to the thread. I’m thinking I should modify the CI quickstart to set JAVA_OPS to not enable the system exit (and use less memory for the JVM than the regular settings)
Trying a change now
x

Xiang Fu

09/25/2021, 2:16 AM
Is it possible to keep the default behavior to make StartZookeeper command ignore this environment variable?
k

Ken Krugler

09/25/2021, 4:36 PM
I fixed it by modifying
.github/workflows/scripts/.pinot_quickstart.sh
to set
JAVA_OPTS
explicitly to something that didn’t set the 
pinot.admin.system.exit
 System property to 
true
, and that seems to work (all tests now pass)