Edgewall Software
Modify

Opened 14 years ago

Closed 14 years ago

#615 closed defect (fixed)

MemoryError in slave when using file attach for an 8MB file.

Reported by: Steven R. Loomis <srl@…> Owned by: osimons
Priority: major Milestone: 0.6
Component: Build slave Version: dev
Keywords: Cc: mpotter@…
Operating System: Other

Description

I was trying a build recipe which attached the built product, an 8MB file, the output of the build product. Maybe I'm Doing It Wrong. Maybe Base64 or something would be better for attaching files?

r902 for master and slave

I added debugging to bitten/util/xmlio.py:43n, i _to_utf8 indicating that len(text) = 8,764,068 FWIW

[DEBUG   ] Sending keepalive
[ERROR   ] Exception raised processing step build. Reraising 
[DEBUG   ] Sending POST request to 'http://example.com/trac/builds/689/keepalive/'
[DEBUG   ] Stopping keepalive thread
[WARNING ] Server returned keepalive error 404: Not Found
[DEBUG   ] Keepalive thread exiting.
[DEBUG   ] Keepalive thread stopped
Traceback (most recent call last):
  File "/opt/freeware/bin/bitten-slave", line 8, in <module>
    load_entry_point('BittenSlave==0.7dev-r902', 'console_scripts', 'bitten-slave')()
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/slave.py", line 554, in main
    exit_code = slave.run()
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/slave.py", line 283, in run
    job_done = self._create_build(url)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/slave.py", line 332, in _create_build
    self._initiate_build(resp.info().get('location'))
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/slave.py", line 346, in _initiate_build
    self._execute_build(build_url, resp)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/slave.py", line 373, in _execute_build
    if not self._execute_step(build_url, recipe, step):
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/slave.py", line 426, in _execute_step
    resp = self.request('POST', build_url + '/steps/', str(xml), {
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 92, in __str__
    self.write(buf)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 198, in write
    Fragment.write(self, out, newlines)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 113, in write
    child.write(out, newlines=newlines)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 198, in write
    Fragment.write(self, out, newlines)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 113, in write
    child.write(out, newlines=newlines)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 198, in write
    Fragment.write(self, out, newlines)
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 118, in write
    out.write(_to_utf8(_escape_text(child)))
  File "/opt/freeware/lib/python2.6/site-packages/BittenSlave-0.7dev_r902-py2.6.egg/bitten/util/xmlio.py", line 43, in _to_utf8
    return text.encode('utf-8')
MemoryError

( The '404' is due to bug #607 )

Attachments (3)

t615-attach_via_temp_file-r902.diff (1.2 KB) - added by osimons 14 years ago.
Attach using temporary file and not stringio.
t615-attachments_multipart_form-r908.diff (25.3 KB) - added by osimons 14 years ago.
Completely redone attachments to attach out-of-band and not as inline xml.
t615-attachments_multipart_form-backport-to-0.6b2.patch (9.9 KB) - added by mazo@… 14 years ago.
backport to 0.6b2, tests omited

Download all attachments as: .zip

Change History (23)

Changed 14 years ago by osimons

Attach using temporary file and not stringio.

comment:1 follow-up: Changed 14 years ago by osimons

  • Milestone changed from 0.6.1 to 0.6
  • Owner set to osimons

Could you perhaps try the attached patch? We do have performance issues with the <attach/> command, and this may help - as opposed to keeping all the file data in memory as we do today.

comment:2 Changed 14 years ago by osimons

Ohhh. Not too impressed with how well I read the description in this ticket... Your memory error all happens at the slave, before even getting the request to the server. We may well do things inefficiently at slave too, so I'll look at this again.

One thing that springs to mind is that as this serialization is progressing, each level of the stack keeps a copy of the base64 encoded string representing the file data. That all adds up to a fair bit of required memory.

comment:3 follow-up: Changed 14 years ago by osimons

Note to self: Check our support for using CDATA content in elements, and exclude such content from being continuously converted - making the presumption of it already being valid UTF-8 compatible content.

comment:4 in reply to: ↑ 3 Changed 14 years ago by osimons

Replying to osimons:

Note to self: Check our support for using CDATA content in elements

No, can't see anything obvious. Hmm...

comment:5 Changed 14 years ago by Steven R. Loomis <srl@…>

I haven't tried the patch, but I see it is for the master, where I haven't had trouble yet (but this is the first time I've tried using the attach step).

This was on Py2.6 on an AIX box.

Changed 14 years ago by osimons

Completely redone attachments to attach out-of-band and not as inline xml.

comment:6 Changed 14 years ago by osimons

In attachment:t615-attachments_multipart_form-r908.diff I have completely redone how attachments are handled. No matter what I tried, the moment the input entered xmlio.parse() memory requirements for both slave and master would jump to excessive amounts when attempting to upload large files.

The new mechanism uses a separate POST request to add the data as a regular multi-part form, complete with form_token and all. The attachments are processed in order but out-of-band, so they will be attached to either build or config before the step is finished on the slave and subsequently processed by master.

No changes needed in configurations - the <attach/> command is unchanged. However, due to form_token support as part of recipe metadata, the PROTOCOL_VERSION needed updating. Both slave and master must be upgraded for the patch to work. Still, better to do this now before we release 0.6b3... :-)

=> A patch well worth testing...

comment:7 in reply to: ↑ 1 Changed 14 years ago by mazo@…

Replying to osimons:

Could you perhaps try the attached patch? We do have performance issues with the <attach/> command, and this may help - as opposed to keeping all the file data in memory as we do today.

I've tried your patch to solve the performance issues with <attach/> (not the one, reported in this ticket).
Maybe I've forgotten something, but applying your patch doesn't change the time/memory consumption required by the master to process a 30MB build attachment.

comment:8 follow-up: Changed 14 years ago by osimons

You got the wrong patch. The first attempt made no real difference - as mentioned in comment:6, handling this by inlining the attachment as base64 in the xml is going to be extremely inefficient and demanding no matter how I cut it.

You want the new monster patch: attachment:t615-attachments_multipart_form-r908.diff

The new patch does not impact the memory usage on master in any noticeable way. The slave reads the file into memory for encoding it as multi-part, but it does not use anywhere near the memory it needed before.

Try patching again, please :-)

comment:9 in reply to: ↑ 8 Changed 14 years ago by mazo@…

Replying to osimons:

You got the wrong patch. The first attempt made no real difference - as mentioned in comment:6, handling this by inlining the attachment as base64 in the xml is going to be extremely inefficient and demanding no matter how I cut it.

You want the new monster patch: attachment:t615-attachments_multipart_form-r908.diff

The new patch does not impact the memory usage on master in any noticeable way. The slave reads the file into memory for encoding it as multi-part, but it does not use anywhere near the memory it needed before.

Try patching again, please :-)

Oops, I read comment:6, but missed the point a bit.
And I was afraid of the second patch, so decided to try the first patch first.:)

Trying the second patch is more difficult because it cannot be applied to a clean 0.6b2 installation.:(
I'm going to attach my effort in backporting it tomorrow and then the results.

Changed 14 years ago by mazo@…

backport to 0.6b2, tests omited

comment:10 follow-up: Changed 14 years ago by osimons

Heh. Appreciate the effort of 'backporting', but for general testing it is much recommended to get the latest from source:/branches/0.6.x and running that - so many things have been fixed since 0.6b2. My patch should apply cleanly on that version.

Anyway, look forward to getting feedback on memory and performance.

comment:11 Changed 14 years ago by Steven R. Loomis <srl@…>

osimons: I've been running from 'trunk' but could switch to the 0.6.x branch. We're in major release mode for which bitten is a major part of our sanity checking. Once things settle down a bit, I can try it. A quick check will be to see if attach dies on the same system or not.

Thanks again for bitten!

comment:12 Changed 14 years ago by osimons

No problem - trunk or 0.6 are both fine. They are currently mirror images of each other, and I actually do all development for trunk (and then we backport to 0.6.x). Patch applies to trunk too.

comment:13 in reply to: ↑ 10 ; follow-up: Changed 14 years ago by mazo@…

Replying to osimons:

Heh. Appreciate the effort of 'backporting', but for general testing it is much recommended to get the latest from source:/branches/0.6.x and running that - so many things have been fixed since 0.6b2. My patch should apply cleanly on that version.

Unfortunately, my environment is near production, so I don't like using live trunk or branch for it.
By the way, are there any scheduled releases in the near future?

Anyway, look forward to getting feedback on memory and performance.

I've tried my "backported" patch, but it causes the build step to fail immediately at <attach/> command.
I'm using HTTPS connection from slave to master, so I can't see sent message right now.
I'm going to take heart and try the source:/branches/0.6.x .

comment:14 in reply to: ↑ 13 Changed 14 years ago by mazo@…

Replying to mazo@…:

I've tried my "backported" patch, but it causes the build step to fail immediately at <attach/> command.
I'm going to take heart and try the source:/branches/0.6.x .

The same problem with source:/branches/0.6.x@909 .

comment:15 follow-up: Changed 14 years ago by osimons

Use -v option when running slave for verbose logging. On master, turn on Trac debug logging. See what you get.

comment:16 in reply to: ↑ 15 ; follow-up: Changed 14 years ago by mazo@…

Replying to osimons:

Use -v option when running slave for verbose logging. On master, turn on Trac debug logging. See what you get.

On slave:

[DEBUG   ] Executing <unbound method Context.attach> with arguments: {'file_': 'build_result.tar.xz', 'resource': 'build'}
[DEBUG   ] Attaching file build_result.tar.xz to build...
[DEBUG   ] Sending POST request to u'https://myserv/projects/mydev/builds/2631/attach/build'
[ERROR   ] Internal error in build step 'pack'
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/bitten/slave.py", line 460, in _execute_step
    self._attach_file(build_url, recipe, output)
  File "/usr/lib64/python2.7/site-packages/bitten/slave.py", line 523, in _attach_file
    'Content-Type': content_type})
  File "/usr/lib64/python2.7/site-packages/bitten/slave.py", line 274, in request
    resp = self.opener.open(req)
  File "/usr/lib64/python2.7/urllib2.py", line 391, in open
    response = self._open(req, data)
  File "/usr/lib64/python2.7/urllib2.py", line 409, in _open
    '_open', req)
  File "/usr/lib64/python2.7/urllib2.py", line 369, in _call_chain
    result = func(*args)
  File "/usr/lib64/python2.7/urllib2.py", line 1181, in https_open
    return self.do_open(httplib.HTTPSConnection, req)
  File "/usr/lib64/python2.7/urllib2.py", line 1142, in do_open
    h.request(req.get_method(), req.get_selector(), req.data, headers)
  File "/usr/lib64/python2.7/httplib.py", line 946, in request
    self._send_request(method, url, body, headers)
  File "/usr/lib64/python2.7/httplib.py", line 987, in _send_request
    self.endheaders(body)
  File "/usr/lib64/python2.7/httplib.py", line 940, in endheaders
    self._send_output(message_body)
  File "/usr/lib64/python2.7/httplib.py", line 801, in _send_output
    msg += message_body
UnicodeDecodeError: 'ascii' codec can't decode byte 0xfd in position 379: ordinal not in range(128)
[DEBUG   ] Sending POST request to 'https://myserv/projects/mydev/builds/2631/steps/'
[WARNING ] Stopping build due to failure

There is nothing interesting on master.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 14 years ago by hodgestar

Replying to mazo@…:

> Replying to [comment:15 osimons]:
> [DEBUG   ] Sending POST request to u'https://myserv/projects/mydev/builds/2631/attach/build'
> ...
>   File "/usr/lib64/python2.7/httplib.py", line 801, in _send_output
>     msg += message_body
> UnicodeDecodeError: 'ascii' codec can't decode byte 0xfd in position 379: ordinal not in range(128)

I'm a bit concerned about the unicode POST URL -- I'm wondering if that is causing the concatenate message to become unicode rather than bytes.

@mazo: Would you mind changing line 522 in slave.py to pass "str(url)" instead of "url" to self.request()?

comment:18 in reply to: ↑ 17 Changed 14 years ago by mazo@…

Replying to hodgestar:

@mazo: Would you mind changing line 522 in slave.py to pass "str(url)" instead of "url" to self.request()?

Looks like, this helped!
The <attach/> completed successfully in a couple of seconds even for a 30MB file.

Thank you for your support!

comment:19 Changed 14 years ago by Mark Potter <mpotter@…>

  • Cc mpotter@… added

comment:20 Changed 14 years ago by osimons

  • Resolution set to fixed
  • Status changed from new to closed

Committed in [913] and merged to 0.6 in [914].

Add Comment

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain osimons.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.