direct_get_suffix_hashes doesn't use replication_ip

Bug #1566395 reported by Sivasathurappan Radhakrishnan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Low
Sivasathurappan Radhakrishnan

Bug Description

direct_client.direct_get_suffix_hashes doesn't use replication_ip and replication_port for doing REPLICATE. Since we have an option of doing replication in separate network, we can add replication_ip and port while creating rings if not it will get filled in with the regular node's ip.

Changed in swift:
assignee: nobody → Sivasathurappan Radhakrishnan (siva-radhakrishnan)
description: updated
description: updated
description: updated
clayg (clay-gerrard)
Changed in swift:
status: New → Confirmed
importance: Undecided → Low
Revision history for this message
clayg (clay-gerrard) wrote :

right now `direct_get_suffix_hashes` is only being used in probe tests so it's not a big deal - but if it's going to get used anywhere else with out surprise side effects this is bad.

Something like this might work for a test

    diff --git a/test/unit/common/test_direct_client.py b/test/unit/common/test_direct_client.py
    index 664a622..d543f0e 100644
    --- a/test/unit/common/test_direct_client.py
    +++ b/test/unit/common/test_direct_client.py
    @@ -19,6 +19,7 @@ import os
     from contextlib import contextmanager
     from hashlib import md5
     import time
    +import pickle

     import mock
     import six
    @@ -97,7 +98,8 @@ def mocked_http_conn(*args, **kwargs):
     class TestDirectClient(unittest.TestCase):

         def setUp(self):
    - self.node = {'ip': '1.2.3.4', 'port': '6000', 'device': 'sda'}
    + self.node = {'ip': '1.2.3.4', 'port': '6000', 'device': 'sda',
    + 'replication_ip': '2.3.4.5', 'replication_port': '7000'}
             self.part = '0'

             self.account = u'\u062a account'
    @@ -732,5 +734,17 @@ class TestDirectClient(unittest.TestCase):
             for line in error_lines:
                 self.assertIn('Kaboom!', line)

    + def test_direct_get_suffix_hashes(self):
    + data = {'a83': 'c130a2c17ed45102aada0f4eee69494ff'}
    + body = pickle.dumps(data)
    +
    + with mocked_http_conn(200, {}, body) as conn:
    + resp_data = direct_client.direct_get_suffix_hashes(
    + self.node, self.part, ['a83'])
    + self.assertEqual(conn.method, 'REPLICATE')
    + self.assertEqual(conn.path, '/sda/0/a83')
    + self.assertEqual(conn.host, '1.2.3.4')
    + self.assertEqual(data, resp_data)
    +
     if __name__ == '__main__':
         unittest.main()

But I feel like that host should be 2.3.4.5, might check port too - are validate the path with multiple hashes

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/301963

Changed in swift:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/301963
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=950b601a9c9e87661e35c6ed7a97ae9611560bc5
Submitter: Jenkins
Branch: master

commit 950b601a9c9e87661e35c6ed7a97ae9611560bc5
Author: Sivasathurappan Radhakrishnan <email address hidden>
Date: Tue Apr 5 22:45:17 2016 +0000

    Modified REPLICATE request to use replication_ip

    direct_client.direct_get_suffix_hashes doesn't use replication ip and
    port for REPLICATE request. Since we have an option of doing
    replication in separate network, we can add replication_ip and port
    while creating rings if not it will get filled in with the regular
    node's ip.

    Change-Id: I34067df27042fc3146b795191ab8043ee1aed3ce
    Closes-Bug:1566395

Changed in swift:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (master)

Reviewed: https://review.openstack.org/302171
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=5d56f40f04fa9211a2e392d667fe395f38d2cca5
Submitter: Jenkins
Branch: master

commit 5d56f40f04fa9211a2e392d667fe395f38d2cca5
Author: Alistair Coles <email address hidden>
Date: Wed Apr 6 11:48:48 2016 +0100

    Make DirectClientException report correct ip and port

    When direct_client.direct_get_suffix_hashes raises a
    DirectClientException the exception message and variables
    should report the replication_ip and replication_port, as
    opposed to the ip and port values reported for all other
    case when the exception is raised.

    Add option to override ip and port reported in
    DirectClientException.

    Also adds unit tests to verify both cases.

    Related-Bug: 1566395
    Change-Id: If3d952847c7199f4e9f6164858085367266386d2

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

Related fix proposed to branch: feature/crypto
Review: https://review.openstack.org/308874

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

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

commit 7d7eaab5afa4c36f0ac8467784138fc423f6ac4f
Author: OpenStack Proposal Bot <email address hidden>
Date: Mon Apr 18 06:31:34 2016 +0000

    Imported Translations from Zanata

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

    Change-Id: I88be5c9dbb1fcc3a15592d42af7160478308b1f4

commit ca304cd08e9f8d37e4027f2f71dd77ebba3a30f9
Author: Samuel Merritt <email address hidden>
Date: Fri Apr 15 17:22:44 2016 -0700

    Ignore negative suffix-byte-range requests.

    If the client asked for "Range: bytes=--123", Swift would respond with
    a 206 and a Content-Length of -123. Now that Range header is ignored
    just like all kinds of other invalid Range headers.

    Change-Id: I30d4522d223076ce342d20c52f57ff0eb2aea1f4
    Closes-Bug: 1571106

commit 746d928a875281a7154dcd438f46a58bbf656db9
Author: Dmitriy Ukhlov <email address hidden>
Date: Fri Apr 8 16:00:16 2016 +0300

    Adds eventlet monkey patching of select module if thread is pathed

    Oslo.messaging pika driver requires patching of select module if thread
    is patched.
    Pika driver uses select call and if it is not patched onsuming messages
    blocks whole eventlet loop

    Closes-Bug: #1570242
    Change-Id: I9756737309f401ebddb7475eb84725f65bca01bf

commit c96d5c671db9c96f65067d93c0a307cf4b7d91b4
Author: oshritf <email address hidden>
Date: Thu Feb 18 14:50:08 2016 +0200

    Per container stat. report

    In addition to the container sync stat. report, keeping per container
    statistics allows administrator with more control over bytes
    transfered over a specific time per user account: The per container stats
    are crucial for billing purposes and provides the operator a 'progress
    bar' equivalent on the container's replication status.

    Change-Id: Ia8abcdaf53e466e8d60a957c76e32c2b2c5dc3fa

commit be84b03a07892f4972dd59309ad261fc72bd7ede
Author: dharmendra <email address hidden>
Date: Thu Apr 14 16:41:09 2016 +0530

    Removing unused clause

    Removing unused clause from common/middleware/dlo.py

    Change-Id: I7de9aaefd203c4f1be00ee74b89b5184fd419469

commit fb3692c9bb662d9cadc4238920f86676857a7f1f
Author: Tim Burke <email address hidden>
Date: Wed Apr 13 11:07:44 2016 -0700

    Don't include conditional headers in SLO HEAD requests

    Previously, attempting to PUT a SLO manifest with `If-None-Match: *`
    would include the header when validating the segments, causing the
    upload to fail.

    Now when SLO validates segments, no conditional headers will be
    included in the HEAD request.

    Change-Id: I03ad454092d3caa73d29e6d30d8033b45bc96136
    Closes-Bug: #1569253

commit 0e7fca576cee81dd6ca8774760cb880c3fff9c1c
Author: Tin Lam <email address hidden>
Date: Fri Apr 8 18:17:44 2016 -0500

    Convert README.md to README.rst

    Change-Id: I223890bd4ffe469becc2127f9362243cdb52b...

Read more...

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

Related fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/323599

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

Reviewed: https://review.openstack.org/323599
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=0330478b70d0a699a0f9c21ef87c7e639d92564b
Submitter: Jenkins
Branch: feature/hummingbird

commit 5fe392b562de3baed080704df433fb392cb4fb31
Author: Ondřej Nový <email address hidden>
Date: Tue May 31 16:25:50 2016 +0200

    Fixed typo

    Change-Id: I7a35c0076360c7a23cf405189828d3c252ec6708

commit b52eccb3b1ea0591f0040587228d3705b5d3f68d
Author: Clay Gerrard <email address hidden>
Date: Wed May 25 11:21:25 2016 -0700

    Clarify overload best practices in admin guide

    Change-Id: Ib7c08bdeab6374771bb8e2b05053e7e16973524d

commit f1fd50723bb84c4941e949895576733f6eb67793
Author: Christian Schwede <email address hidden>
Date: Wed May 25 09:53:31 2016 +0200

    Add dispersion --verbose example to admin guide

    Change-Id: I5f9cacedde2a329332ccf744800b6f2453e8b28e

commit b3ab715c055283ccfea9a504d6da20741d82e7ad
Author: Matthew Oliver <email address hidden>
Date: Wed May 25 14:35:54 2016 +1000

    Add ring-builder dispersion command to admin guide

    This change updates the admin guide to point out the dispersion command
    in swift-ring-builder and mentions the dispersion verbose table to make
    it more obvious to operators.

    Change-Id: I72b4c8b2d718e6063de0fdabbaf4f2b73694e0a4

commit fb7a8e9ab7596a36a6992a3a8f8c6d005a2c2829
Author: Tim Burke <email address hidden>
Date: Tue May 24 13:37:58 2016 -0700

    Add links to mitaka install guides

    Change-Id: I62331923751c521daded4468b5cc5f03655226bc

commit e09c4ee7800e82aa09ca2f6ae375420b766182a4
Author: Tim Burke <email address hidden>
Date: Fri Apr 29 12:12:00 2016 -0500

    Allow concurrent bulk deletes

    Before, server-side deletes of static large objects could take a long
    time to complete since the proxy would wait for a response to each
    segment DELETE before starting the next DELETE request.

    Now, operators can configure a concurrency factor for the slo and bulk
    middlewares to allow up to N concurrent DELETE requests. By default, two
    DELETE requests will be allowed at a time.

    Note that objects and containers are now deleted in separate passes, to
    reduce the likelihood of 409 Conflict responses when deleting
    containers.

    Upgrade Consideration
    =====================
    If operators have enabled the bulk or slo middlewares and would like to
    preserve the prior (single-threaded) DELETE behavior, they must add the
    following line to their [filter:slo] and [filter:bulk] proxy config
    sections:

       delete_concurrency = 1

    This may be done prior to upgrading Swift.

    UpgradeImpact
    Closes-Bug: 1524454
    Change-Id: I128374d74a4cef7a479b221fd15eec785cc4694a

commit 226557afc42c245e050d84162497f46341407ef7
Author: Tim Burke <email address hidden>
Date: Thu May 19 18:55:40 2016 -0700

    Turn on H703, so our translators don't punch us

    Change-Id: I4ce3068f79563e4d4296c6e1078bc12f0cf84c96
    Related-Bug: 1559431

commit 7b706926a8ed5bbcec3a678e868e301c9a6ed8f1
Author: Alistair Coles <email address hidden>
Date: Mon May ...

tags: added: in-feature-hummingbird
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/swift 2.8.0

This issue was fixed in the openstack/swift 2.8.0 release.

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.