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

Esrally command broken on Windows because of invalid default logging configuration #532

Closed
michardy opened this issue Jul 11, 2018 · 5 comments

Comments

@michardy
Copy link

Rally version (get with esrally --version):
Latest? (esrally --version fails)

Invoked command:
any esrally command (for example esrally configure)

Configuration file (located in ~/.rally/rally.ini)):
Not yet created

JVM version:
1.8

OS version:
Windows 10 Pro version 1803

Description of the problem including expected versus actual behavior:
The esrally command fails to run due to invalid escape in %userprofile%\.rally\logging.json on line 23. This error occurs because esrally attempts to mix Windows and Linux paths and does not escape the \ in Windows paths. Ideally the generated logging.json should use the local systems path separator and properly escape it.

Steps to reproduce:

  1. Open an administrator prompt in c:\Program Files\Python36\Scripts
  2. Run .\pip install esrally
  3. Run .\esrally configure (or any other esrally command)
  4. It errors due to an invalid escape on line 23

Provide logs (if relevant):
Line 23 reads as follows:
"filename": "C:\Users\mhardy\.rally\logs/rally.log",

Traceback (most recent call last):
  File "c:\program files\python36\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "c:\program files\python36\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Program Files\Python36\Scripts\esrally.exe\__main__.py", line 9, in <module>
  File "c:\program files\python36\lib\site-packages\esrally\rally.py", line 487, in main
    log.configure_logging()
  File "c:\program files\python36\lib\site-packages\esrally\log.py", line 90, in configure_logging
    logging.config.dictConfig(load_configuration())
  File "c:\program files\python36\lib\site-packages\esrally\log.py", line 61, in load_configuration
    return json.load(f)
  File "c:\program files\python36\lib\json\__init__.py", line 299, in load
    parse_constant=parse_constant, object_pairs_hook=object_pairs_hook, **kw)
  File "c:\program files\python36\lib\json\__init__.py", line 354, in loads
    return _default_decoder.decode(s)
  File "c:\program files\python36\lib\json\decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "c:\program files\python36\lib\json\decoder.py", line 355, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Invalid \escape: line 23 column 22 (char 688)
@michardy michardy changed the title Esrally command broken on Windows because of invalid default configuration Esrally command broken on Windows because of invalid default logging configuration Jul 11, 2018
@danielmitterdorfer
Copy link
Member

Thank you for your interest in Rally @michardy! At the moment, Rally only works on Unix. We do mention it in the docs but I see that we missed to state this clearly in the README as well.

That does not mean though that you cannot benchmark an Elasticsearch cluster running on Windows. It is just the machine where Rally is installed (which generates the load) needs to run on Unix.

As an immediate measure, we will ensure that the operating system requirements are stated clearly in the README. But we might look into Windows support only at a later point in time.

@littlesnitch
Copy link

littlesnitch commented Dec 29, 2018

@danielmitterdorfer, @michardy,
I still was digging a little about this topic.
I found after installation the logging configuration template is being written into directory
\Lib\site-packages\esrally\resources\logging.json

The culprits in this JSON file are the two places for filename under
rally_log_handler
rally_profile_handler
I just replaced there the slashes with backslashes.
Then after starting esrally under windows, this template was copied into path %USERPROFILE%.rally and rally started working.
If I understand the whole thing correctly, then the file logging_1_0_0.json is again used for generating the logging.json, which holds the following content in my case for filename:
rally_log_handler ${LOG_PATH}/rally.log
rally_profile_handler ${LOG_PATH}/profile.log
And the solution could be to correct the variable $LOG_PATH.
A fix could be in this block of log.py
`def remove_obsolete_default_log_config():
"""
Log rotation is problematic because Rally uses multiple processes and there is a lurking race condition when
rolling log files. Hence, we do not rotate logs from within Rally and leverage established tools like logrotate for that.

Checks whether the user has a problematic out-of-the-box logging configuration delivered with Rally 1.0.0 which
used log rotation and removes it so it can be replaced by a new one in a later step.
"""
log_config = log_config_path()
if io.exists(log_config):
    source_path = io.normalize_path(os.path.join(os.path.dirname(__file__), "resources", "logging_1_0_0.json"))
    with open(source_path, "r", encoding="UTF-8") as src:
        contents = src.read().replace("${LOG_PATH}", default_log_path())
        source_hash = hashlib.sha512(contents.encode()).hexdigest()
    with open(log_config, "r", encoding="UTF-8") as target:
        target_hash = hashlib.sha512(target.read().encode()).hexdigest()
    if source_hash == target_hash:
        os.rename(log_config, "{}.bak".format(log_config))

def install_default_log_config():
"""
Ensures a log configuration file is present on this machine. The default
log configuration is based on the template in resources/logging.json.

It also ensures that the default log path has been created so log files
can be successfully opened in that directory.
"""
log_config = log_config_path()
if not io.exists(log_config):
    io.ensure_dir(io.dirname(log_config))
    source_path = io.normalize_path(os.path.join(os.path.dirname(__file__), "resources", "logging.json"))
    with open(log_config, "w", encoding="UTF-8") as target:
        with open(source_path, "r", encoding="UTF-8") as src:
            contents = src.read().replace("${LOG_PATH}", default_log_path())
            target.write(contents)
io.ensure_dir(default_log_path())`

and replace it with this block
at the beginning:
import platform
and the places before:
`def remove_obsolete_default_log_config():
"""
Log rotation is problematic because Rally uses multiple processes and there is a lurking race condition when
rolling log files. Hence, we do not rotate logs from within Rally and leverage established tools like logrotate for that.

Checks whether the user has a problematic out-of-the-box logging configuration delivered with Rally 1.0.0 which
used log rotation and removes it so it can be replaced by a new one in a later step.
"""
log_config = log_config_path()
log_platform = platform.system()
if io.exists(log_config):
    source_path = io.normalize_path(os.path.join(os.path.dirname(__file__), "resources", "logging_1_0_0.json"))
    with open(source_path, "r", encoding="UTF-8") as src:
        if log_platform == "Windows":
            contents = src.read().replace("${LOG_PATH}", default_log_path()).replace('\\','/')
        else:
            contents = src.read().replace("${LOG_PATH}", default_log_path())
        source_hash = hashlib.sha512(contents.encode()).hexdigest()
    with open(log_config, "r", encoding="UTF-8") as target:
        target_hash = hashlib.sha512(target.read().encode()).hexdigest()
    if source_hash == target_hash:
        os.rename(log_config, "{}.bak".format(log_config))

def install_default_log_config():
"""
Ensures a log configuration file is present on this machine. The default
log configuration is based on the template in resources/logging.json.

It also ensures that the default log path has been created so log files
can be successfully opened in that directory.
"""
log_config = log_config_path()
log_platform = platform.system()
if not io.exists(log_config):
    io.ensure_dir(io.dirname(log_config))
    source_path = io.normalize_path(os.path.join(os.path.dirname(__file__), "resources", "logging.json"))
    with open(log_config, "w", encoding="UTF-8") as target:
        with open(source_path, "r", encoding="UTF-8") as src:
            if log_platform == "Windows":
                contents = src.read().replace("${LOG_PATH}", default_log_path()).replace('\\','/')
            else:
                contents = src.read().replace("${LOG_PATH}", default_log_path())
            target.write(contents)
io.ensure_dir(default_log_path())

This could be one approach at generation level. Or otherwise the log_config_path and default_log_path and maybe other places with.replace('\','/')`

What do you mean?

@littlesnitch
Copy link

Ok, think I get it, now are next difficulties with tracks, after fixing this, there were errors finding the templates. So it seems there is gonna be a lot to do to fix this...

@danielmitterdorfer
Copy link
Member

@littlesnitch thanks for looking into this. Apart from what you found out, there are other problems on Windows as well. I am specifically thinking of the internal communication between Rally components. As you can distribute Rally, e.g. to support use-cases that require to generate load from multiple machines, we use an actor system internally. I did some tests a while ago and ran into all sorts of stability issues with it. We will likely do something about this in the mid-term but this will require quite a few changes.

@littlesnitch
Copy link

Ok, thank you for you answer. I understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants