Monday, 2019-11-25

*** rcernin has quit IRC00:17
*** rcernin has joined #openstack-swift00:17
*** baojg has joined #openstack-swift01:08
seongsoochomorning o/01:23
*** nanzha has joined #openstack-swift01:56
*** diablo_rojo__ has joined #openstack-swift03:41
*** diablo_rojo__ has quit IRC04:31
*** nanzha has quit IRC04:40
*** nanzha has joined #openstack-swift04:41
*** nanzha has quit IRC05:00
*** nanzha has joined #openstack-swift05:02
openstackgerritCharles Hsu proposed openstack/python-swiftclient master: WIP: Support uploading Swift symlinks without content.  https://review.opendev.org/69421105:10
*** jistr has quit IRC05:51
*** jistr has joined #openstack-swift05:56
*** nanzha has quit IRC06:00
*** jistr has quit IRC06:06
*** jistr has joined #openstack-swift06:06
*** nanzha has joined #openstack-swift06:12
*** nanzha has quit IRC06:24
*** nanzha has joined #openstack-swift06:36
*** rcernin has quit IRC07:09
*** nanzha has quit IRC07:22
*** joeljwright has joined #openstack-swift07:45
*** ChanServ sets mode: +v joeljwright07:45
*** nanzha has joined #openstack-swift08:00
*** tkajinam has quit IRC08:06
*** tesseract has joined #openstack-swift08:16
*** ccamacho has joined #openstack-swift08:25
*** ccamacho has quit IRC08:25
*** ccamacho has joined #openstack-swift08:25
*** rpittau|afk is now known as rpittau08:33
*** rdejoux has joined #openstack-swift08:49
*** nanzha has quit IRC09:00
*** nanzha has joined #openstack-swift09:01
*** noonedeadpunk is now known as noonedeadpunk_09:56
*** noonedeadpunk_ is now known as noonedeadpunk09:56
*** jistr is now known as jistr|afk09:58
*** takamatsu has quit IRC10:05
*** takamatsu has joined #openstack-swift10:06
*** jistr|afk is now known as jistr10:22
*** nanzha has quit IRC10:34
*** nanzha has joined #openstack-swift10:38
*** nanzha has quit IRC11:06
*** nanzha has joined #openstack-swift11:06
*** pcaruana has joined #openstack-swift11:14
*** openstack has joined #openstack-swift11:53
*** ChanServ sets mode: +o openstack11:53
*** irclogbot_3 has joined #openstack-swift11:53
*** nanzha has quit IRC12:03
*** nanzha has joined #openstack-swift12:04
*** nanzha has quit IRC12:18
*** nanzha has joined #openstack-swift12:29
*** nanzha has quit IRC12:36
*** nanzha has joined #openstack-swift12:37
*** nanzha has quit IRC13:12
*** zaitcev_ has joined #openstack-swift13:14
*** ChanServ sets mode: +v zaitcev_13:14
*** nanzha has joined #openstack-swift13:17
*** zaitcev__ has quit IRC13:18
*** mvkr has quit IRC13:19
*** nanzha has quit IRC13:24
*** nanzha has joined #openstack-swift13:30
*** nanzha has quit IRC13:56
*** nanzha has joined #openstack-swift13:58
*** diablo_rojo__ has joined #openstack-swift14:40
*** Crisp has joined #openstack-swift14:47
*** Crisp has quit IRC14:47
*** diablo_rojo__ has quit IRC14:57
*** diablo_rojo__ has joined #openstack-swift14:57
*** mvkr has joined #openstack-swift15:06
*** diablo_rojo__ is now known as diablo_rojo15:31
*** openstackstatus has joined #openstack-swift16:15
*** ChanServ sets mode: +v openstackstatus16:15
*** nanzha has quit IRC16:18
*** rpittau is now known as rpittau|afk16:40
timburkegood morning16:42
*** takamatsu has quit IRC17:27
*** FlorianFa has quit IRC17:29
*** rdejoux has quit IRC17:30
claygtimburke: I think we talked about this -> https://bugs.launchpad.net/swift/+bug/185388417:31
openstackLaunchpad bug 1853884 in OpenStack Object Storage (swift) "s3api doesn't log internal requests" [Undecided,New]17:31
claygI verified SwiftStack clusters already log s3api subrequests by default ๐Ÿ‘17:31
*** takamatsu has joined #openstack-swift17:32
timburkeyeah, i like that description at the end a *whole* lot better -- that's the option we *really* want17:34
timburkei should check what happens when you've got `s3_acl` disabled... i think maybe it just does the right thing?17:35
claygtimburke: yeah so https://review.opendev.org/#/c/673682/25/test/unit/common/middleware/s3api/test_obj.py@1272 - that's  a great question17:45
patchbotpatch 673682 - swift - s3api: Implement object versioning API - 25 patch sets17:45
clayginstead of trying to DELETE an *object* that doesn't exist - we try to DELETE a *version* that doesn't exist?17:45
claygthe problem I was debugging is the shortcircut of the HEAD 404 was preventing the DELETE request from making its way to swift object versioning middleware17:46
claygas the code reads now s3api is doing a HEAD obj?version-id=XXX request - and if that 404s we...17:46
claygstill do the DELETE obj?version-id=XXX request?  I'm assuming swift object versioning would do the right thing, but maybe we want the old short circut logic?17:47
claygit's not like we need to generate a delete marker in this case17:47
timburkeright -- i was mainly thinking about making sure that delete-with-version-id works correctly when the object doesn't exist in the primary container17:51
timburkepretty trivial to do with a func test, too -- write obj, version-unaware delete it, version-aware delete the data17:53
*** tesseract has quit IRC17:57
claygoh, well - for a version aware DELETE we don't really make a request to the un-versoined symlink thing... SHOULD WE?!18:05
timburke...then how do we know whether or not we should lay down a tombstone?18:06
claygyou mean how does swift object versioning know?  s3api just looks at the swift object versioning response18:06
claygand we don't emulate the backing swift object versioning subreuqests18:06
clayg... in these unittests18:07
timburkehmm... maybe i was thinking of a version-aware delete for a *version* that doesn't exist... i forget now... :-/18:09
*** diablo_rojo has quit IRC18:09
timburkeyeah yeah! in case we find ourselves with a broken symlink -- so we still show the version in the listing -- if the client's just trying to clean up everything, will they be able to do so with a version-aware DELETE for the busted version? significantly, what will the result of that head be at https://review.opendev.org/#/c/673682/24..25/swift/common/middleware/s3api/controllers/obj.py ?18:12
patchbotpatch 673682 - swift - s3api: Implement object versioning API - 25 patch sets18:12
timburkei'm pretty sureit all works fine -- but i'm not sure that we have unit tests to explicitly test it18:13
*** diablo_rojo has joined #openstack-swift18:14
claygso that HEAD request there, when you're doing  DELETE?versionId=XXX goes to HEAD?version-id=XXX18:18
timburkewhich will try to follow the symlink and 40418:19
timburkeraising the NoSuchKey error, clearing the query param -- yeah?18:19
claygno, I think a HEAD?version-id will just goto the data in the versions container, which... I mean it COULD 404 if you tried to delete a version that doesn't exist?18:19
claygI am definately not following you, because I don't see how the symlink is involved on a DELETE?versionId request (from s3api's perspective)18:20
claygif the question is how is swift object versioning handling a DELETE?version-id request when the is_latest symlink 404s... that's probalby just a different set of unittests18:21
timburkeso i guess i've got two worries: first, the dangling symlink problem (which should hopefully get cleaned up), and the delete for a non-existent version (where the symlink should get left alone)18:21
timburkeyeah, i suppose so18:21
claygmaybe that's why I'm confused - these unittests are for s3api, but yeah as soon as I address your comments - the func tests will be AWESOME!18:22
claygtimburke: do you have any fixes in your tree for the comments left on the latest null namespace patch?18:24
timburkenot yet -- push away18:25
claygok, i'm pushing ontop of 543e2a (patch set 34) so none of your comments will be disturbed18:26
timburkeeither way18:26
openstackgerritClay Gerrard proposed openstack/swift master: s3api: Implement object versioning API  https://review.opendev.org/67368218:26
openstackgerritClay Gerrard proposed openstack/swift master: New Object Versioning mode  https://review.opendev.org/69596618:26
claygtimburke: well it looks like a number of the issues you pointed at on the null namepace patch aren't directly related to the reaper issue you're testing18:32
claygI'll just keep them in mind for now as I really want to work on some more s3api func tests - but at somepoint I'd be happy to go back to the null namespace patch and try and tighten up some stuff18:33
timburke๐Ÿ‘18:34
openstackgerritTim Burke proposed openstack/swift master: s3api: Fix prefix/delimiter/marker quoting  https://review.opendev.org/69578118:34
timburkespeaking of s3, i *think* that guy should actually pass tests now18:34
openstackgerritTim Burke proposed openstack/swift master: s3api: Fix prefix/delimiter/marker quoting  https://review.opendev.org/69578118:56
timburkehmm... should direct_client always set X-Backend-Allow-Reserved-Names? i can't think of a good reason *not* to off hand...19:02
claygwell, i was thinking about listings19:04
claygbut i guess now that we've dis-allowed mixing null and non-null names ๐Ÿค”19:05
claygwell... only in containers19:06
claygtimburke: you pointed out that doing that prevents us from adding null names (container or obj) to the expirer queue19:07
claygthat may be a mistake19:07
claygor rather "that might be something we decide to roll back at some point" (where "at some point" might be like... now)19:07
timburkeyeah -- the expirer/reconciler implications of that are a little troubling19:08
claygi think originally we'd thought of that - and then somehow later forgot (but it's good you remembed!)19:09
timburkei suppose the other option is to revisit https://review.opendev.org/#/c/517389/ sooner rather than later... and rework the interfaces proposed in https://review.opendev.org/#/c/517389/46/swift/common/task_queue.py19:12
patchbotpatch 517389 - swift - Add object-expirer new mode to execute tasks from ... - 46 patch sets19:12
patchbotpatch 517389 - swift - Add object-expirer new mode to execute tasks from ... - 46 patch sets19:12
claygi don't see how that's feasible in our timeframe - or even really how it helps19:15
clayg... but I'd be happy to rollback the "no user names in null containers" restriction19:15
timburkeit'd mostly be about accepting as forced-work changing our naming convention for work queues19:16
claygIs it possible weโ€™re clever enough that a HEAD?version-id request to a delete marker would 404? Iโ€™d that... โ€œcorrectโ€?19:47
timburkei think i did something for that...19:48
timburkelook for "Well, except for some delete marker business..." -- i think it maybe needs a Content-Location header, though...19:49
*** diablo_rojo has quit IRC20:11
timburkeok, so i keep finding py3 rabbit-holes as i poke around... things that are logically separate from the reserved-namespace patch, but that i want to fix up as part of it... should i keep rolling those in, or should i get a new patch (or two, or...) ahead of everything? i *do* kinda want to backport some of this to train...20:25
timburkeok, so it looks like the answer for https://review.opendev.org/#/c/682138/34/swift/common/utils.py@190 is that otherwise, we hit a non-obvious circular import error, where constraints tries to load request_helpers (so it can get RESERVED_BYTE), which tries to load wsgi, which tries to load constraints but then complains that it doesn't have a MAX_HEADER_SIZE attribute...20:43
patchbotpatch 682138 - swift - Allow internal clients to use reserved namespace - 34 patch sets20:43
timburkeso flip side: is there any compelling reason not to have all the stuff currently in https://review.opendev.org/#/c/682138/34/swift/common/request_helpers.py over in utils instead? or possibly even constraints...20:43
patchbotpatch 682138 - swift - Allow internal clients to use reserved namespace - 34 patch sets20:43
claygno, none at all - that seemed like a great idea21:14
timburkeclayg, so i think the expirer/reconciler problem is deeper than just allowing mixed user and reserved names...21:21
claygneat!21:21
timburkethe problem is that we get queue objects like <policy>:/<acct>/<cont>/<obj>, and if <cont> or <obj> are reserved names, the whole thing vails to validate because it doesn't start with \x0021:23
claygoh right21:23
timburkethough... maybe it'll be ok if we just fix up the container layer to accept that format? like, the subdir problem shouldn't really apply to the reconciler...21:25
claygtrue!21:26
claygmaybe limit to dot accounts21:27
timburkeyeah, seems reasonable...21:27
timburkemaybe we default-in the header if it's a system account?21:28
claygwe could probably also just move the whole thing get_reserved_name(policy, path)21:28
timburkeyeah, i was thinking about that, too...21:28
timburkewe get the problem of needing to look in multiple places, though21:29
claygmaybe we need policy, path = split_reserved_name(name, limit=1)21:29
claygright, it'd be a little crufty - but i'd image we could get it to all work out ... the reconciler at least is pretty atomic21:30
claygthe expirer I guess has that property of removing entries when the expir name is updated ๐Ÿค”21:31
*** rcernin has joined #openstack-swift21:31
claygok, so maybe just letting the "invalid" names into the internal containers *is* better21:31
*** biyiklioglu has joined #openstack-swift21:32
*** pcaruana has quit IRC21:38
*** biyiklioglu has quit IRC21:38
*** diablo_rojo has joined #openstack-swift21:53
timburkeprobe tests aren't really opinionated about rsync vs ssync, yeah? i've got a pretty small change to add reserved names to test_replication_servers_working.py but idk that it's really worth running if we're using rsync in the gate (which i'm pretty sure we are)22:12
timburkestill, good to see ssync working!22:12
timburkei'd just tweak the test_reconstructor_* tests, but they have a lot more places where they want to use swiftclient...22:13
timburkemy delta: http://paste.openstack.org/show/786691/22:14
claygtimburke: that's fabulous!!!22:22
*** threestrands has joined #openstack-swift22:22
timburkeclayg, so... should i include it in the patches i'm about to push? or skip it since the gate'll just be using rsync?22:23
timburkealso, i lied a little -- needed to add create_account to internal_client and set allow_account_management=true in probe/common22:24
claygyour question makes me think I don't understand some implication of the change ๐Ÿค”22:25
claygeven if we're not able to hit that functionality in the gate it's still useful to have it *available* right?  what's the down side?22:25
claygthe gate will only test what it's configired to test right?22:26
claygand we want null namespace to work with rsync too!  ๐Ÿ˜22:26
claygi'm also ready to push up some tests & fixes for delete markers22:26
timburkeit doesn't have much implication -- i'd just *expect* that rsync should work, since it's just looking at fs paths, none of which involve reserved chars. so we'd be running an extra test, taking extra time, and i would expect it to catch pretty much *exactly* the same problems as the normal test (at least, when run in the gate)22:27
timburkeeh, i'll go ahead and include it22:28
timburkei'm realizing that even if the reserved namespace patch doesn't think about container-sync, the versioning patch may have to...22:29
openstackgerritTim Burke proposed openstack/swift master: Allow internal clients to use reserved namespace  https://review.opendev.org/68213822:30
mattoliveraumorning22:30
timburkeo/22:30
timburkeclayg, should UPDATE do any per-item name validation?22:31
timburkemaybe that's a way out of our work-queue problem... it certainly doesn't do any such validation *today*22:32
*** zaitcev_ is now known as zaitcev22:41
zaitcevclayg, do you happen to have a vacuum script laying around by chance? I want a script that 1) locks the directory of the container, 2) runs VACUUM, 3) increments some timestamp or other so that replication does not overwrites it with raw rsync, 4) unlocks.22:42
zaitcevclayg, I think you're the only guy around who may have something like that in his cupboard.22:43
mattoliverauI always wondered if we should run a vacuum (or have an option too) when we're about to rsync a db accross. Cause then 1) it'll be less data and 2) would add some auto vacuuming.. sometimes (when rings change)22:45
timburkemattoliverau, i was just reading your response on https://bugs.launchpad.net/swift/+bug/1691648 :-)22:46
openstackLaunchpad bug 1691648 in OpenStack Object Storage (swift) "empty 5GB container DB" [Medium,Confirmed]22:46
mattoliverautold you I always wondered ;)22:46
zaitcev5GB haha.... (bitter laught)22:47
zaitcevI have a customer who have a 300GB container22:48
mattoliveraulol22:48
zaitcevI'm pretty desperate at this point.22:49
zaitcevIt's probably not at all empty.22:52
mattoliverauhmm, what rsyncs are you worried about. if the db doesn't exist, it'll be rsynced.. if it does but has a "large" disparity then it'll rsync_then_merge which rsyncs to a new location. then merge and a rename/mv22:57
mattoliverauso not sure what timestamp to change.22:57
timburkei forget which db (local or remote) is used as the base for the result in rsync_then_merge -- i think it might be the remote, with the assumption that the local ought to be small22:58
timburkeso that'd be what i'd worry about22:58
mattoliverauoh I thought it was the opposite.23:00
mattoliverauso it stayed out of the way. the copy send over is written in tmp. then the existing is merged into then renamed to replace the orig.23:01
mattoliveraubut that's from memory, that might be wrong.23:01
mattoliverauhttps://www.sqlite.org/lang_vacuum.html interesting a VACUUM is a write, so should block other writes. So holds a write lock. Maybe if one is done at a time then it's fine.23:02
mattoliverautho also, it copies rows to another temp db first.. so you might need up to twice as much space to run a vacuum.. that could be a problem.23:02
mattoliverauauto-vacuum seems to be a little different.23:03
timburkeyep, looks like the remote is used as the base: https://github.com/openstack/swift/blob/2.23.0/swift/common/db_replicator.py#L987-L100823:03
mattoliverauok yeah the remote.. and I just reread and realised what I said wasn't the opposite, but the same.. lol. /me reads good :P23:04
*** tkajinam has joined #openstack-swift23:09

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!