Skip to content

Fix running pulsar-perf in docker image after #11285 changes #13022

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.

Sign up for GitHub

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

Jump to bottom
Closed
lhotari wants to merge 1 commit into from
Closed

Fix running pulsar-perf in docker image after #11285 changes #13022

lhotari wants to merge 1 commit into from

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Nov 29, 2021

Motivation

Running pulsar-perf in docker fails with this error:

root@3b36a0ea475b:/pulsar# ./bin/pulsar-perf produce 
[0.000s][error][logging] Error opening log file 'logs/pulsar_gc_14.log': No such file or directory
[0.000s][error][logging] Initialization of output 'file=logs/pulsar_gc_%p.log' using options 'filecount=10,filesize=20M' failed.
Invalid -Xlog option '-Xlog:gc*:logs/pulsar_gc_%p.log:time,uptime:filecount=10,filesize=20M', see error log for details.
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

Modifications

@lhotari
Fix running pulsar-perf in docker image after apache#11285 changes
b0b2baf 
- create /pulsar/logs directory since it's now required for
  running pulsar-perf after apache#11285 changes
@lhotari lhotari added type/bug The PR fixed a bug or issue reported a bug doc-not-needed Your PR changes do not impact docs labels Nov 29, 2021
@lhotari lhotari self-assigned this Nov 29, 2021
@lhotari lhotari added this to the 2.10.0 milestone Nov 29, 2021
@lhotari lhotari mentioned this pull request Nov 29, 2021
Merged
@lhotari lhotari requested a review from eolivelli November 29, 2021 11:14
nicoloboschi
nicoloboschi approved these changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

LGTM

@eolivelli
Copy link
Contributor

eolivelli commented Nov 29, 2021

I looks like that this patch is only fixing the docker images.

what happens to people who try to download the pulsar binary tarball and try to run pulsar-perf ?
is it broken ?

@lhotari
Copy link
Member Author

lhotari commented Nov 29, 2021

I looks like that this patch is only fixing the docker images.

what happens to people who try to download the pulsar binary tarball and try to run pulsar-perf ? is it broken ?

Valid point. yes, the same problem exists in the binary tarball.

@lhotari
Copy link
Member Author

lhotari commented Nov 29, 2021

The reason why running the broker doesn't fail is the fact that the logs directory is always created

pulsar/bin/pulsar

Lines 292 to 294 in d3d580f

if [ ! -d "$PULSAR_LOG_DIR" ]; then
mkdir -p "$PULSAR_LOG_DIR"
fi

It feels wrong that pulsar-perf would have GC logging enabled by default.

@lhotari
Copy link
Member Author

lhotari commented Nov 29, 2021

I'm closing this PR and replacing with a broader refactoring of how the parameters are passed.

@lhotari lhotari closed this Nov 29, 2021
@lhotari
Copy link
Member Author

lhotari commented Nov 29, 2021

This is a better fix #13025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicoloboschi nicoloboschi nicoloboschi approved these changes

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@BewareMyPower BewareMyPower Awaiting requested review from BewareMyPower

@eolivelli eolivelli Awaiting requested review from eolivelli

Assignees

@lhotari lhotari

Labels

doc-not-needed Your PR changes do not impact docs type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Milestone

2.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@lhotari @eolivelli @nicoloboschi