Use of GreenAsyncPile can lose txn_id logging

Bug #1409302 reported by Darrell Bishop
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
In Progress
Medium
janonymous

Bug Description

...if you don't do what swift.proxy.controllers.base.Controller.make_requests() does, which is to send self.app.logger.thread_locals down into the function being run in the green threads (self._make_request() in this case), where it can be reinstalled into self.app.logger inside each new greenthread.

AFAICT, this bug only affects the one other usage of GreenAsyncPile in Swift: swift.proxy.controllers.obj.ObjectController._get_put_responses() which calls an inline function, get_conn_response() using a GreenAsyncPile. Sure enough, the log line that can be emitted by get_conn_response() does not have a transaction ID on it.

  Jan 9 09:01:27 container-server: "PUT /d22/162825/..." 201 - "PUT http://localhost/d19/98559/..." "tx634a7ed2349d4b5e8c58d-0054b00967" "object-server 11284" 0.0004 "-" 21679
  Jan 9 09:01:27 container-server: "PUT /d13/162825/..." 201 - "PUT http://localhost/d23/98559/..." "tx634a7ed2349d4b5e8c58d-0054b00967" "object-server 21454" 0.0005 "-" 21337
  Jan 9 09:01:27 object-server: "PUT /d23/98559/..." 201 - "PUT http://localhost/v1/..." "tx634a7ed2349d4b5e8c58d-0054b00967" "proxy-server 25895" 0.1066 "-" 21454Jan 9 09:01:27 object-server: "PUT /d19/98559/..." 201 - "PUT http://localhost/v1/..." "tx634a7ed2349d4b5e8c58d-0054b00967" "proxy-server 25895" 0.1663 "-" 11284
  Jan 9 09:01:27 swift: - - 09/Jan/2015/17/01/27 PUT /v1/... HTTP/1.0 201 - Worker - 7327 - - tx634a7ed2349d4b5e8c58d-0054b00967 - 0.4966 - - 1420822887.361249924 1420822887.857897043
  Jan 9 09:01:28 container-server: "PUT /d18/162825/..." 201 - "PUT http://localhost/d15/98559/..." "tx634a7ed2349d4b5e8c58d-0054b00967" "object-server 21695" 0.0008 "-" 11219
  Jan 9 09:01:28 object-server: "PUT /d15/98559/..." 201 - "PUT http://localhost/v1/..." "tx634a7ed2349d4b5e8c58d-0054b00967" "proxy-server 25895" 1.3962 "-" 21695
  Jan 9 09:02:27 swift: ERROR with Object server 192.168.1.141:6000/d15 re: Trying to get final status of PUT to /v1/... Timeout (5s)

CVE References

clayg (clay-gerrard)
Changed in swift:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
clayg (clay-gerrard) wrote :

I wonder if we could make a factory for GreenAsyncPile's on a base wsgi application class such that when you get a pile it has the property of always passing in it's app's logger's thread locals into a wapped version of your spawned callable that fixes the locals inside of the spawn before calling your function...

Or maybe a TransactionIDPreservingGreenAsyncPile class that takes a logger... that's probably simpler - we might even be able to audit for usage of GreenAsyncPile...

janonymous (janonymous)
Changed in swift:
assignee: nobody → janonymous (janonymous)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (master)

Fix proposed to branch: master
Review: https://review.openstack.org/266239

Changed in swift:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift (master)

Change abandoned by janonymous (<email address hidden>) on branch: master
Review: https://review.openstack.org/266239
Reason: I think i squashed the changes but parent changed, Please find the patch after squashing : https://review.openstack.org/#/c/278176/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/278176
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=4906b4c431edc436f165b163a228b0a221950c79
Submitter: Jenkins
Branch: master

commit 4906b4c431edc436f165b163a228b0a221950c79
Author: Janonymous <email address hidden>
Date: Tue Jan 12 12:50:43 2016 +0530

    Fix missing txn_id logs in GreenAsyncPile's spawned functions

    This commit ensures that the logger thread_locals
    value is passed to and set in _get_conn_response methods
    executed in a green thread.

    Added partial bug tag because in bug description a more
    relevant fix is suggested which would fix the bug completely
    but for now this makes sense to add this commit for logging.

    Co-Authored-By: Clay Gerrard <email address hidden>
    Change-Id: I13bbf174fdca89318d69bb0674ed23dc9ec25b9a
    Partial-Bug: #1409302

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/crypto)

Fix proposed to branch: feature/crypto
Review: https://review.openstack.org/281742

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/crypto)
Download full text (11.1 KiB)

Reviewed: https://review.openstack.org/281742
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=be911d8abf9122bf69064e82296348b62973ec2f
Submitter: Jenkins
Branch: feature/crypto

commit e46d6b17251ab538595b1deb5581360a286b8fd1
Author: Christian Schwede <email address hidden>
Date: Wed Feb 17 20:47:26 2016 +0100

    Add note on using printable chars for swift_hash_path_suffix/prefix

    Using unprintable characters for swift_hash_path_prefix/suffix might lead to
    hard problems when parsing of these values changes, for example due to newer
    Python versions or changes in the parsers itself. Let's avoid this and add a
    note that deployers should use printable strings for these values.

    Change-Id: I976982b753b6af831ab91d7190f50f8f15bf73bf

commit b8fa48080812ab0f6bac7ae19c0290da08f6a6e4
Author: Donagh McCabe <email address hidden>
Date: Thu Feb 4 16:19:13 2016 +0000

    Added links to API reference and usage guides

    Added a link to the API reference (where headers and query
    strings are documented; click the "detail" button to see).
    Also added a reference to Swift section of the OpenStack end
    user guide. This contains some additional details about the API.

    No attempt was made to reconcile duplicate information. Instead
    this patch links documents that might otherwise be overlooked.
    However, I fixed text, originally in a table, that had become
    garbled in a prior patch.

    Change-Id: I0910cbeb0c8bffc00e510f35585603e7b7a67790

commit aa7204d106ae33eba9219514f31f008510c9db53
Author: Kazuhiro MIYAHARA <email address hidden>
Date: Fri Feb 12 20:48:34 2016 +0900

    Remove '#! /usr/bin/env python' from unexecutable files

    'cli/recon.py' and 'cli/ring_builder_analyzer.py' have '#! /usr/bin/env
    python' in spite of they don't have execute permissions. This patch
    removes '#! /usr/bin/env python' from them.

    Change-Id: I1917ccc84b1673af3d862be1796f54595f94c5ca

commit 8eb30afd7c39f6e6ba031f72c6a61f45b28d37f6
Author: Tim Burke <email address hidden>
Date: Tue Feb 16 10:25:39 2016 -0800

    Static methods should be @staticmethods

    Change-Id: Ifee5d68e00bbb3571aaac885cdd7490c79732985

commit 42f4b3fc1e69a046c1e70e73afe847e4423fa2da
Author: Christian Schwede <email address hidden>
Date: Tue Feb 16 10:08:55 2016 +0000

    Add SwiftHLM to associated projects

    Change-Id: I5ea3de34e61f22abec803b35fec8adde18a793e9

commit 025ebf2901d5a972796b73538bb306847d7d06b6
Author: Ankur Gupta <email address hidden>
Date: Wed Feb 10 14:36:16 2016 -0600

    Docstring omission in class BaseDiskFileManager.

    Added documentation for missing Docstring variables.

    Change-Id: I29a53b8141c5607815f234a6123e2289200bca34

commit 4906b4c431edc436f165b163a228b0a221950c79
Author: Janonymous <email address hidden>
Date: Tue Jan 12 12:50:43 2016 +0530

    Fix missing txn_id logs in GreenAsyncPile's spawned functions

    This commit ensures that the logger thread_locals
    value is passed to and set in _get_conn_response methods
    executed in a green thread.

    Added partial bug tag because in bu...

tags: added: in-feature-crypto
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/hummingbird)

Fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/290148

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/hummingbird)
Download full text (71.7 KiB)

Reviewed: https://review.openstack.org/290148
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=0f7f1de233919a0b046349a3e31ae7fc8675a1c5
Submitter: Jenkins
Branch: feature/hummingbird

commit d6b4587a554b51ba733b151e0d924735b63d07e0
Author: Olga Saprycheva <email address hidden>
Date: Tue Mar 8 10:57:56 2016 -0600

    Removed redundant file for flake8 check

    Change-Id: I4322978aa20ee731391f7709bbd79dee140fc703

commit 643dbce134140530eef2ae62c42fef1107f905ed
Author: OpenStack Proposal Bot <email address hidden>
Date: Tue Mar 8 06:35:49 2016 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I96b8ff1287bf219c5f8d56a3a4868c1063a953f9

commit 83713d37f0331c5ce9d377f4b4e8724551ae30ca
Author: Daisuke Morita <email address hidden>
Date: Mon Mar 7 18:30:47 2016 -0800

    Missing comments for storage policy parameter

    There are missing comments about storege_policy_index so appropriate
    comments are added.

    Change-Id: I3de3f0e6864e65918ca1a13cce70f19c23d295f5

commit 2cff2dec3d1c4588f5103e39679c43b3dded6dcb
Author: Olga Saprycheva <email address hidden>
Date: Fri Mar 4 15:19:39 2016 -0600

    Fixed pep8 and flake8 errors in doc/source/conf.py and updated flake8 commands in tox.ini to test it.

    Change-Id: I2add370e4cfb55d1388e3a8b41f688a7f3f2c621

commit 043fbca6d08648baa314ea2236f1ccdca8785f16
Author: Christian Schwede <email address hidden>
Date: Fri Mar 4 09:33:17 2016 +0000

    Remove Erasure Coding beta status from docs

    This removes notes stating support for Erasure coding as beta. Questions
    regarding the stability of EC are coming up regularly, and are often referring
    to the docs that state EC as still in beta.

    Besides this, a note marking statsd support as beta has been removed as well.

    Change-Id: If4fb6a5c4cb741d42953db3cee8cb17a1d774e15

commit 09c73b86e9255f28fbd4cf571a52c17d549a8f9a
Author: Pete Zaitcev <email address hidden>
Date: Thu Mar 3 10:24:28 2016 -0700

    Fix a crash in exception printout

    Says the number of arguments does not match the number of '%'.

    Change-Id: I8b5e395a07328fb9d4ac7a19f8ed2ae1637bee3b

commit fad5fabe0a22e8a86635a66523dd3d3d3b1fa705
Author: Tim Burke <email address hidden>
Date: Thu Mar 3 15:07:08 2016 +0000

    During functional tests, 404 response to a DELETE is successful

    Previously, we would only consider 204 responses successful, which would
    cause some spurious gate failures, such as

    http://logs.openstack.org/66/287666/3/check/gate-swift-dsvm-functional/c6d2673/console.html#_2016-03-03_13_41_07_846

    Change-Id: Ic8c300647924352a297a2781b50064f7657038b4

commit e91de49d6864b3794f8dc5acd9c1bf0c2f7409d1
Author: Alistair Coles <email address hidden>
Date: Mon Aug 10 10:30:10 2015 -0500

    Update container on fast-POST

    This patch makes a number of changes to enable content-type
    metadata to be updated when using the fast-POST mode of
    operation, as proposed in the associated spec ...

tags: added: in-feature-hummingbird
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.