[00:36:15] simonwilkinson: any chance you're around? [00:37:56] I've got a question about fixing up a gerrit push, but I'll ask in the morning [00:41:02] --- dev-zero@jabber.org has left [01:10:53] --- dev-zero@jabber.org has become available [01:51:11] --- haba has become available [04:57:27] --- mmeffie has become available [04:57:59] --- haba has left [04:58:28] --- mmeffie has left [05:02:20] --- haba has become available [05:12:43] jake, all you need to do is push to e.g. refs/changes/317 instead of refs/for/master [06:21:51] --- haba has left [06:22:28] --- haba has become available [06:32:00] --- reuteras has left [06:32:33] --- deason has become available [07:07:17] --- mmeffie has become available [07:15:59] --- matt has become available [07:31:18] derrick, oh, that makes more sense. Thanks, is there a way to delete those two? [07:32:11] there should be a button to 'abandon' a change [07:33:07] yeah, when viewing a change, it's next to the 'publish comments' button [07:33:57] --- deason has left [07:37:06] alright, I'll do that instead [07:40:54] --- deason has become available [08:16:48] --- stevenjenkins has left [09:16:20] --- dev-zero@jabber.org has left [10:38:56] --- Russ has become available [10:45:36] hm, in SRXAFS_Rename, we appear to delete and recreate the .. entry when we rename a directory, even if the parent dir doesn't change [10:45:40] isn't that unnecessary? [11:15:22] yeah, seems likely that could be optimized. [11:15:50] In the steady state, maybe, but I think there are some broken cases that can occur where .. is wrong or doesn't already exist (think reattach during salvage) [11:16:20] Plus it means that renaming a directory always bumps its DV. You might want to think carefully before changing that; there might be something that depends on it. [11:16:58] yes, that's what I was wondering [11:17:43] I was surprised that the dv was bumped in that case... but other things may be surprised when suddenly it's not [11:21:54] The OpenAFS CM does not currently seem to care -- if the thing being renamed is a directory and the parent changes, it explicitly invalidates the dcache entry corresponding to the directory being removed _and_ invalidates its '..' entry in the dnlc, but doesn't take special action to update the .. entry locally or bump the DV. If the parent does not change, it doesn't even do that. [11:22:27] see src/afs/VNOPS/afs_vnop_rename.c:afsename(), particularly near the end. [11:24:28] Note that the rename call doesn't return a new FetchStatus on the object being renamed, but if it is a directory (and thus .. was updated and the DV changed) then the fileserver will break a callback on the directory being renamed. [11:24:34] In fact, I think that is a bug... [11:25:54] > if (fileptr->disk.type == vDirectory) /* if a dir moved, .. changed */ > BreakCallBack(client->host, &fileFid, 0); That last 0 should be 1, so the host doing the Rename gets a callback break, too. Otherwise it will not know that the target dir changed, because there is no code (as far as I can see) which marks the directory being renamed as out-of-date (not statd) [11:28:30] the host doing the rename needs a callback? [11:29:35] --- stevenjenkins has become available [11:32:03] It shouldn't, in that it should know the object being renamed might change. But I don't understand what the if(!oneDir) code at the end of afsrename() is doing; it certainly doesn't mark the vnode as not statd and in need of an update. [11:33:24] Yes. It does seem that the client should make the assumption. [11:45:56] Right. But it's not entirely clear to me that it _does_, in all the places where it should. [11:46:09] s/places/cases/ [11:49:21] Patch needed. Are you going to do it? I can. [11:51:04] Or, Andrew, were you going to do it? (wrt just what jhutz is pointing out above) [11:53:15] I think I want to understand better whether we have a buggy client before producing a server patch to work around it. [11:53:20] er, which? the BCB with flag=1? or client-side? [11:53:25] I think the client should do it. [11:53:31] That is the bug, isn't it? [11:53:38] I certainly don't understand the client-side code here enough to fix it [11:54:09] the trivial case on a client of just moving a dir to a different dir doesn't show a bug, btw, at least as far as I can tell [11:54:27] I'll try to reproduce. I have some more XCB rename work to do anyway. [11:54:52] If there are existing buggy clients (which I'm not yet convinced of), then IMHO we should make the server do the BCB with flag=1 in addition to fixing the clients, because upgrading a few servers is much easier than upgrading all the clients in the universe. [11:54:57] I can do the patch so we don't change .. when not neeed easily, but it's not clear that it won't break things [11:55:07] and it's not exactly a critical problem [11:55:11] What do you mean by "doesn't show a bug" ? [11:55:40] I mean, the .. entry on the client that did the rename appears to be correct [11:56:02] jhutz: that does make sense--but how long would we continue such a behavior? [11:56:02] I think it's fine to not change .. when we don't need to. It's fairly clear, to me at least, that the existing OpenAFS client doesn't expect the DV to bump in that case. Maybe we should poll other clients. [11:56:20] Forever, for clients that don't claim XCB support. [11:56:46] Because those clients will be assert(fixed)? [11:57:09] Right. As will clients that use any new Rename RPC we invent down the road, even if they don't claim XCB support. [11:57:16] Ok. [11:57:58] --- haba has left [11:58:00] --- abo has left [11:58:06] --- stevenjenkins has left [11:58:47] --- haba has become available [11:58:47] --- abo has become available [12:01:15] --- stevenjenkins has become available [12:01:33] one thing I thought was odd, though not necessarily buggy; when we do unnecessarily update .., the callback for the dir isn't broken, I don't think [12:01:43] but does it matter? the .. entry should be the same, except for the dv [12:03:34] Yes it is [12:03:54] I think [12:04:09] conditional looks like if (fileptr->disk.type == vDirectory) [12:04:17] Oh, hm, no, we don't. [12:04:17] inside a if (oldvptr != newvptr) { [12:04:52] But that's OK. The contents haven't changed, so it doesn't matter that clients don't know the DV has changed. They'll figure it out next time there's a real change to that directory. [12:05:29] okay, good [12:05:58] or, hm, well, it _shouldn't_ change; if the .. entry was wrong pre-rename and it was fixed, clients would still have the wrong one [12:06:17] but if I change SAFSS_Rename to not update the .. entry anyway, that will still obviously be the case [12:06:31] --- haba has left [12:06:50] Right. [12:10:52] So you're convinced there's no reason to have clients re-stat? [12:12:39] There may not be an observable issue with old clients, but I'm inclined to make the vcache statle. [12:12:40] stale [12:15:01] i think it should be simply so we have up to date dv info [12:15:06] check [12:15:13] instead of waiting to discover it later [12:15:38] But I think the assertion is, there is no need to BCB for legacy cases [12:16:01] i believe that is true [13:18:50] --- Simon Wilkinson has become available [13:21:51] Which reminds me. I really must make rename do a sillyrename when it clobbers an existing file. I've been planning to do that for months now. [14:11:24] sillyrename? [14:13:39] e.g. if you remove a file you have open, we create one of the ._afsXXX files so it doesn't go away [14:13:54] we don't yet do the same thing if you mv-clobber a file [14:14:02] ah [14:16:26] It's why the blogbench benchmark won't run on AFS. [14:26:22] --- dev-zero@jabber.org has become available [16:11:31] --- matt has left [16:22:10] --- Jeffrey Altman has become available [16:29:41] --- Jeffrey Altman has left: Replaced by new connection [17:38:22] --- matt has become available [17:38:30] --- matt has left [18:15:55] --- deason has left [18:41:42] --- dev-zero@jabber.org has left: Replaced by new connection [18:41:44] --- dev-zero@jabber.org has become available [18:46:05] --- deason has become available [20:09:20] --- Russ has left: Replaced by new connection [20:09:20] --- Russ has become available [21:14:03] --- deason has left [21:14:35] --- deason has become available [21:36:39] --- deason has left [22:11:04] --- reuteras has become available