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

max_event_size does not always trim large events. #50062

Closed
whytewolf opened this issue Oct 15, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@whytewolf
Copy link
Contributor

commented Oct 15, 2018

Description of Issue/Question

max_event_size Does not always trim long events.

If the event has a key:value pair where the value is a dict and not a string than the dict_trim will run sys.getsizeof on the dict. This returned size is much lower than the issue size even in cases where it shouldn't be.

However just serializing that dict and running the trim on it also doesn't seem like the right answer as that could be trimming important information used for parsing the event data.

I'm not sure what the correct course is for this. But I can see this causing headaches for very large state runs.

Setup

Reducing this down to a more simple test can be done with the following.

create 2 largeish random files and convert them to base64 so that we don't trigger diff binary mode.
for i in 1 2; do dd if=/dev/urandom of=/tmp/testfile bs=512 count=10000; base64 /tmp/testfile > /srv/salt/testfile.${i}; rm -f /tmp/testfile; done

then alternate between these 2 states.

test1:
  file.managed:
    - name: /tmp/testfile
    - source: salt://testfile.1

test2:
  file.managed:
    - name: /tmp/testfile
    - source: salt://testfile.2

This will create a huge return dict inside the return data. which will get sent back to the master as an job event will not be trimmed by dict_trim.

Steps to Reproduce Issue

See above

Versions Report

Salt Version:
           Salt: 2018.3.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Aug  4 2017, 00:39:18)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.4.1708 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-693.11.6.el7.x86_64
         system: Linux
        version: CentOS Linux 7.4.1708 Core

although dict_trim really hasn't changed since 2016 so this should happen in all versions since then.

@whytewolf whytewolf added the ZD label Oct 15, 2018

@whytewolf whytewolf changed the title max_event_size does not always trim log events. max_event_size does not always trim large events. Oct 15, 2018

@whytewolf

This comment has been minimized.

Copy link
Contributor Author

commented Oct 15, 2018

zd-2887

@garethgreenaway

This comment has been minimized.

Copy link
Member

commented Nov 3, 2018

Made some progress on this one today, here is a preview from the salt event runner:

salt/job/20181102175212462507/ret/moriarty      {
    "_stamp": "2018-11-03T00:52:12.465772",
    "arg": [
        "50062"
    ],
    "cmd": "_return",
    "fun": "state.sls",
    "fun_args": [
        "50062"
    ],
    "id": "moriarty",
    "jid": "20181102175212462507",
    "out": "highstate",
    "retcode": 0,
    "return": {
        "file_|-test1_|-/tmp/testfile_|-managed": {
            "__id__": "test1",
            "__run_num__": 0,
            "__sls__": "50062",
            "changes": {
                "diff": "VALUE_TRIMMED"
            },
            "comment": "File /tmp/testfile updated",
            "duration": 84.922,
            "name": "/tmp/testfile",
            "pchanges": {},
            "result": true,
            "start_time": "17:52:12.227672"
        },
        "file_|-test2_|-/tmp/testfile_|-managed": {
            "__id__": "test2",
            "__run_num__": 1,
            "__sls__": "50062",
            "changes": {
                "diff": "VALUE_TRIMMED"
            },
            "comment": "File /tmp/testfile updated",
            "duration": 70.977,
            "name": "/tmp/testfile",
            "pchanges": {},
            "result": true,
            "start_time": "17:52:12.312783"
        }
    },
    "tgt": "moriarty",
    "tgt_type": "glob"
}
@garethgreenaway

This comment has been minimized.

Copy link
Member

commented Jan 9, 2019

Closing this out, if the problem persists please comment & we'll re-open the issue or feel free to open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.