#1212 closed defect (fixed)

Repair used default shares.happy

Reported by: eurekafag Owned by: kmarkley86
Priority: critical Milestone: 1.8.1
Component: code-network Version: 1.8.0
Keywords: regression repair servers-of-happiness Cc: amontero@…
Launchpad Bug:

Description (last modified by eurekafag)

I've tried to repair a file and got:

<class 'allmydata.interfaces.UploadUnhappinessError'>: shares could be
placed on only 5 server(s) such that any 3 of them have enough shares
to recover the file, but we were asked to place shares on at least 7
such servers. (placed all 10 shares, want to place shares on at least
7 servers such that any 3 of them have enough shares to recover the
file, sent 7 queries to 5 peers, 7 queries placed some shares, 0
placed none (of which 0 placed none due to the server being full and 0
placed none due to an error))

Everything worked fine on 1.7.1 and shares.happy = 3 (didn't changed it after upgrade). So I did a little investigation and found the problem. It's immutable/repairer.py, line 60:

happy = upload.BaseUploadable.default_encoding_param_happy

Why do we use default happy here? It definitely should be read from the config. I didin't dig further but replaced it with ugly hack:

happy = 3 #upload.BaseUploadable.default_encoding_param_happy

...and the problem has gone! Repairing works with just 6 servers online.

Attachments (5)

1212.dpatch (3.2 KB) - added by kevan at 2010-09-28T04:21:33Z.
1212.patch (2.7 KB) - added by eurekafag at 2010-09-28T07:28:07Z.
1212.darcspatch.txt (3.2 KB) - added by zooko at 2010-09-29T05:12:24Z.
1212again.darcspatch.txt (3.2 KB) - added by zooko at 2010-09-29T05:14:57Z.
1212.darcs.patch (3.2 KB) - added by zooko at 2010-09-29T05:18:25Z.

Download all attachments as: .zip

Change History (69)

comment:1 Changed at 2010-09-25T15:47:48Z by eurekafag

  • Description modified (diff)

comment:2 follow-up: Changed at 2010-09-27T03:19:23Z by kevan

  • Version changed from 1.8β to 1.8.0

We eventually decided that this should be 0 when designing and implementing Servers of Happiness; see #778, around comment 45 for the discussion on that.

If no one has claimed this ticket by Tuesday, I'll fix it.

(I'm also setting the version to 1.8.0; if I understand your mailing list message, that's what your client is running. Feel free to change it back if I'm wrong. :-)

comment:3 in reply to: ↑ 2 Changed at 2010-09-27T05:29:29Z by eurekafag

Yes, it's 1.8.0 but there was no such a version in a drop-down list when I created the ticket. This issue is important because it breaks the repair feature on small grids with <7 nodes online. And even more important because repairing is needed more often on such small networks.

comment:4 Changed at 2010-09-27T20:10:21Z by kevan

  • Owner set to kevan

Changed at 2010-09-28T04:21:33Z by kevan

comment:5 follow-up: Changed at 2010-09-28T04:22:36Z by kevan

  • Keywords review-needed added
  • Owner kevan deleted

I think that the patch in attachment:1212.dpatch fixes this issue.

Changed at 2010-09-28T07:28:07Z by eurekafag

comment:6 in reply to: ↑ 5 ; follow-up: Changed at 2010-09-28T07:29:29Z by eurekafag

Why do you provide patches in dpatch format? It's debian only and I didn't find sources in google to compile it for OpenSuSE (which is RPM-based). So I patched by hand and here is unified diff patch: attachment:1212.patch

comment:7 Changed at 2010-09-28T07:46:25Z by eurekafag

Looks like this version works, thanks.

comment:8 in reply to: ↑ 6 Changed at 2010-09-28T13:55:22Z by zooko

Replying to eurekafag:

Why do you provide patches in dpatch format? It's debian only and I didn't find sources in google to compile it for OpenSuSE (which is RPM-based). So I patched by hand and here is unified diff patch: attachment:1212.patch

That was actually a darcs patch rather than a debian patch. I didn't realize that "dpatch" stood for "debian patch". I've updated Patches to suggest that people named their darcs patches thing.darcspatch.txt instead of thing.dpatch.

comment:9 Changed at 2010-09-29T05:05:22Z by zooko

  • Keywords reviewed added; review-needed removed
  • Milestone changed from undecided to soon
  • Resolution set to fixed
  • Status changed from new to closed

Reviewed and applied in ec4f87a98c034dac, thanks! By the way, I think we should do more work here. This patch corrects the regression from v1.7.1 to v1.8.0 (introduced in 797828f47fe1aa44), in that v1.7.1 would repair with a servers-of-happiness (H) of 0 and v1.8.0 would repair with an H of 7. While I agree that this was a regression and that we should put it back to 0, I actually think that the old behavior of 0 was wrong and that we should have been using "currently configured H" instead!

That is: if you have configured your servers-of-happiness H to be 3, like eurekafag did, and the number of servers currently reachable on your grid is 2, and you do a repair, then I think the repair should stop with an explicit error message instead of proceeding and then giving you a report at the end that mentions (if you know how to read it) that it actually only put the shares onto 2 servers.

(In other words, I think I was wrong when I suggested letting repairer use H==0 in comment:48:ticket:778. Or at least, what we did then was to keep the behavior of repairer from v1.6 when we made v1.7, but what I'm suggesting to do now is improve that behavior for the next release.)

Kevan, David-Sarah, Brian, eurekafag: do you agree? If so, let's open a new ticket saying to make the H used by repair be the same as the H that would be used by an upload. (Also in the new code we should make the H value be a parameter passed to the repairer instead of letting the repairer query the node-wide configuration. This is in keeping with CodingStandards regarding configuration and will facilitate some possible future work where people can pass explicit K, M, and H for a given upload or repair, e.g. as options to the tahoe put command line or optional fields in the WUI.

comment:10 Changed at 2010-09-29T05:11:23Z by zooko

Just testing syntax highlighting by uploading an attachment named "thing.darcspatch.txt"...

Changed at 2010-09-29T05:12:24Z by zooko

Changed at 2010-09-29T05:14:57Z by zooko

Changed at 2010-09-29T05:18:25Z by zooko

comment:11 Changed at 2010-09-29T07:06:15Z by eurekafag

I do agree that zero happiness should be changed to H. There is no need to create a ticket because I've mentioned that already: It definitely should be read from the config. The temporary solution is nice but not complete to close this ticket.

comment:12 follow-up: Changed at 2010-09-29T20:12:23Z by kevan

If we do that, we lose the property that the repairer will always try to place whichever shares are missing onto *some* storage servers, even if the end result isn't optimally distributed.

If I have a cron job that does a deep repair of my rootcap, and the rootcap or some other important dircap or filecap only has k or k+1 shares available, and it is stored on a grid with a lot of churn, I probably care more about the fact that there are more than a few shares of that cap around than I do about where they are, and I certainly wouldn't want the repairer to not even bother generating new ones because it couldn't satisfy my distribution criteria; IOW, I'm better off with more shares that are poorly distributed than I am with no repair action (I'm oversimplifying, and it depends on the specific situation, but having more shares will make things better in some situations and generally won't make things worse, AFAICT without doing the math).

On the other hand, I think that the repairer should definitely tell the user whether the file is distributed correctly or not, and an exception message certainly does that. I can also make my node's repair go for broke with share regeneration by changing the value of happiness in tahoe.cfg to be 0. This is a chore, but it means that people who really want the repairer to try to place new shares regardless of where can still get that behavior.

Maybe the best approach is to fix #614 with this in mind. The repairer could regenerate and try to place all of the missing shares, as it does now, but also tell the caller (in the post repair results) whether the repair was ultimately successful or not based on how the shares are distributed, using the client's configured happiness value for that check.

Edit: I didn't read Zooko's comment closely enough. Is what I describe in the third paragraph what the repairer already does? If so, what don't you like about that?

Last edited at 2010-09-29T20:15:44Z by kevan (previous) (diff)

comment:13 in reply to: ↑ 12 Changed at 2010-09-30T01:17:35Z by zooko

Replying to kevan:

If we do that, we lose the property that the repairer will always try to place whichever shares are missing onto *some* storage servers, even if the end result isn't optimally distributed.

Doesn't this mean that H is effectively 0 for you when you are doing this?

I can also make my node's repair go for broke with share regeneration by changing the value of happiness in tahoe.cfg to be 0. This is a chore, but it means that people who really want the repairer to try to place new shares regardless of where can still get that behavior.

Right. If you want this behavior, set H==0. If you want the other behavior (abort the repair) set H to something else. With the v1.7.1 behavior and the current trunk behavior (since 20100927200102-b8d28-9111a341188a4264e5070f91b52364a2addcb3dc), setting H in your tahoe.cfg has no effect on repairer behavior—repairer always acts as though H==0.

Maybe the best approach is to fix #614 with this in mind. The repairer could regenerate and try to place all of the missing shares, as it does now, but also tell the caller (in the post repair results) whether the repair was ultimately successful or not based on how the shares are distributed, using the client's configured happiness value for that check.

Oh, good catch. Yes, if we fix #614 then repairer would be using H (during the check/verify step) to determine whether or not to trigger a repair. Once it triggered the repairer, then it could also use H to determine whether to abort the repair, or it could instead treat H as effectively 0 for the purpose of the repair.

Now that I've thought about it more and read your comments, Kevan, I think I agree that we should have the latter behavior, as long as we fix #614 so that the output reported by the repairer can be easily understood by the user as indicating "unhealthy" when the servers of happiness is less than H.

Oh, in fact, what I really want is for repairer to proceed and to do its best even if it knows that it can't reach servers of happiness greater than or equal to H (instead of aborting the way uploader does), but then to return a failure result saying that it wasn't able to repair the file back to health.

Does that make sense?

Okay, I'm done changing my mind for the moment. What do you think?

Edit: I didn't read Zooko's comment closely enough. Is what I describe in the third paragraph what the repairer already does? If so, what don't you like about that?

Sorry: I don't understand this question. Hopefully I answered it above.

comment:14 follow-ups: Changed at 2010-10-01T01:08:55Z by davidsarah

This is how I think the repairer should work (I think this is violently agreeing with Zooko's comment:13, but with more detail):

  • let k and N be the shares-needed and total number of shares for this file, and let H be the happiness threshold read from tahoe.cfg.
  • if there are fewer than k connected servers, report that the repair failed completely.
  • construct a server map for this file by asking all connected servers which shares they have. (In the case of a mutable file, construct a server map for the latest retrievable version.)
  • construct a maximum matching M : server -> share, of size |M|, for this file (preferring to include servers that are earlier on the permuted list when there is a choice).
  • while |M| < N, and we have not tried to put shares on all connected servers:
    • pick a share not in M, and the server not in M that is next on the permuted list (wrapping around if necessary). Try to extend M by putting that share onto that server.
  • if |M| < k, report that the repair failed completely. If k <= |M| < H, report that the file is retrievable but unhealthy. In any case report what |M| is.

(The while loop should be done in parallel, with up to N - |M| outstanding requests.)

comment:15 Changed at 2010-10-01T04:20:12Z by zooko

  • Keywords regression repair added

comment:16 Changed at 2010-10-01T04:20:24Z by zooko

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:17 Changed at 2010-10-07T22:04:24Z by zooko

  • Milestone changed from soon to 1.8.1

comment:18 Changed at 2010-10-13T23:12:49Z by kevan

comment:14 seems sensible to me.

I'm not sure if milestone 1.8.1 is about the little regression that I submitted a patch for, or the broader, likely to be fixed by #614 issue of how the repairer should work. If the latter, then 1.8.1 might be a little optimistic; fixing #614 correctly will require (unless I'm missing something obvious) a decent chunk of engineering, since the immutable file repairer is currently very simple. I would at least be more confident in my ability to get #614 done by 1.9.0 than by 1.8.1.

comment:19 in reply to: ↑ 14 ; follow-up: Changed at 2010-10-14T02:55:55Z by zooko

Replying to davidsarah:

This is how I think the repairer should work (I think this is violently agreeing with Zooko's comment:13, but with more detail):

...

  • if there are fewer than k connected servers, report that the repair failed completely.

Why this step?

comment:20 in reply to: ↑ 19 Changed at 2010-10-14T02:57:54Z by davidsarah

Replying to zooko:

Replying to davidsarah:

This is how I think the repairer should work (I think this is violently agreeing with Zooko's comment:13, but with more detail):

...

  • if there are fewer than k connected servers, report that the repair failed completely.

Why this step?

Just a shortcut; this case would fail in the last step anyway.

Replying to davidsarah: [...]

  • while |M| < N, and we have not tried to put shares on all connected servers:
    • pick a share not in M, and the server not in M that is next on the permuted list (wrapping around if necessary). Try to extend M by putting that share onto that server.

A small refinement of this step would be that once |M| >= H, we could allow placing the remaining N-H shares on servers that are already in the matching, if we're unable to place them on servers that are not in the matching.

comment:21 Changed at 2010-10-14T02:58:58Z by zooko

Replying to davidsarah:

  • if |M| < k, report that the repair failed completely. If k <= |M| < H, report that the file is retrievable but unhealthy. In any case report what |M| is.

Okay, but why this one?

We definitely need to classify health into several types: unrecoverable, 100% (|M|>=N), and servers-of-happiness-satisfying (|M|>=H) (needs a better name! "healthy" ?).

Do we also need another type to show that servers_of_happiness >= K?

Last edited at 2010-10-14T03:03:50Z by zooko (previous) (diff)

comment:22 Changed at 2010-10-14T03:10:36Z by zooko

I think we should distinguish between levels of happiness at which the uploader or repairer will

  • (a) abort the upload or repair,
  • (b) treat the file as "unhealthy" which means
    • (b.1) a check-and-repair will trigger the repair step and
    • (b.2) a check results or a repair results will show the file (with clear, noticeable presentation) as being in danger ),
  • (c) treat the file as "perfectly healthy" which means that a repair would not make any change to it.

One of the questions in this ticket -- comment:21 -- is whether (a) should trigger when |M| < K or not. Sometimes people would rather that the uploader/repairer get the file out there, even if all the shares are on a single server! Other times people might prefer that the uploader/repairer avoid wasting bandwidth on that and instead stop and raise the alarm.

#614 is all about whether (b) should trigger when |M| < N (current behavior) or |M| < H (proposed new behavior).

comment:23 Changed at 2010-10-14T03:11:59Z by zooko

Oh, what I just proposed in comment:22 is a significant new behavior if we allow the level of happiness that triggers (a) to be different than the level of happiness that triggers (b)! Currently uploader/repairer aborts the upload or repair if it knows that it cannot achieve "health", i.e. |M| >= H. There are even unit tests to ensure that buildbot will go red if uploader/repairer proceeds to do an upload when it can't reach that level of happiness. :-)

comment:24 Changed at 2010-10-14T03:21:26Z by davidsarah

Ah, I confusingly said "|M| < k" when I actually meant to say "the file is not retrievable". (It might be retrievable if there are >= k shares, but on less than k distinct servers.)

I think we should only abort a repair if the file is not retrievable (in which case we can't repair it anyway).

Last edited at 2010-10-14T03:21:57Z by davidsarah (previous) (diff)

comment:25 Changed at 2010-10-14T03:24:59Z by davidsarah

Hmm, why shouldn't a check-and-repair always try to restore a file to happiness N? The only reason I can think of is that it might result in redundant shares if there are a few servers that are sometimes disconnected, but wouldn't that tend to stablise after a few repair cycles?

comment:26 Changed at 2010-10-14T04:40:38Z by zooko

I guess something that I haven't made up my mind about yet is how repair jobs (either tahoe repair command on the cli or clicking on the "check-and-repair" button on the wui) should handle the case that the upload/repair fails, or partially fails on some of the files.

Should it proceed to completion, generate a report saying to what degree each attempt to repair a file succeeded, and exit with a "success" code (i.e. exit code 0 from tahoe repair), or should it abort the attempt to repair this one file, and should it also abort any other file repair attempts from the current deep-repair job?

For example, suppose you ask it to repair a single file with K=3, H=7, N=10, and it finds out that there are only two storage servers currently connected. One storage server has 3 shares and the other has 0. Then should it abort the upload immediately? Or should it upload a few shares (3?) to the second storage server which currently has none, and then report to you that the file is still unhealthy?

Here is one set of principles to answer this question (not sure if this is the best set):

  1. Idempotence if you run an upload-or-repair job, and it does some work (uploads some shares), and then you run it again when nothing has changed among the servers (there are no servers that joined or left and none of them acquired or lost shares), then the second run will not upload any shares.
  1. Forward progress if you run a repair job (not necessarily an upload job!), and it is possible for it to make |M| greater than it was before, then it will do so.

If we use these principles then we give up on an alternate principle:

  1. Network efficiency if you run an upload or repair job, and it is impossible for it to make |M| >= H, then it does not use any bulk network bandwidth. (Also, if it looks like it is possible at first, but after it has started uploading then one of the servers fails and it becomes impossible, then it aborts right then and does not use any more of your network bandwidth.)

I think people (including me) intuitively wanted principle 3 for uploads, but now that we are thinking about repairs instead of uploads we intuitively want principle 2.

comment:27 follow-up: Changed at 2010-10-14T04:53:40Z by zooko

One possibility would be to make the behavior of uploader different than of repairer. Perhaps people prefer for their initial uploads to fail quickly and network-efficiently (principle 3) if they won't be able to achieve happiness level of H, but prefer for their repairs to proceed and do their best (principle 2). However, making the two behave differently would make things more complicated in the source code and would also make things more complicated in the usage, because principle 1 -- idempotence -- would not apply to "first upload and then repair" or "first repair and then upload". Sometimes an upload would abort itself and return failure but then a subsequent repair would do a lot of work to make progress, or a repair would do a lot of work to make progress but then an upload would abort itself and return failure.

Unless we are really sure that we need to support two different modes, I would prefer to err on the side of simplicity and find a mode that is good enough for both upload and repair. One good way to estimate "complication in usage" is to think how much documentation we would need to write to explain the different behavior of upload and repair in the different cases. :-)

comment:28 in reply to: ↑ 27 Changed at 2010-10-14T05:01:11Z by davidsarah

Replying to zooko:

One possibility would be to make the behavior of uploader different than of repairer. Perhaps people prefer for their initial uploads to fail quickly and network-efficiently (principle 3) if they won't be able to achieve happiness level of H, but prefer for their repairs to proceed and do their best (principle 2). However, making the two behave differently would make things more complicated in the source code and would also make things more complicated in the usage, because principle 1 -- idempotence -- would not apply to "first upload and then repair" or "first repair and then upload". Sometimes an upload would abort itself and return failure but then a subsequent repair would do a lot of work to make progress, or a repair would do a lot of work to make progress but then an upload would abort itself and return failure.

Unless we are really sure that we need to support two different modes, I would prefer to err on the side of simplicity and find a mode that is good enough for both upload and repair.

I'm not sure that two different modes would add much complexity. Almost all of the code would be shared, and the upload/repair flag would just enable the fast abort in the upload case.

comment:29 Changed at 2010-10-29T06:03:34Z by zooko

At some point, possibly in email to tahoe-dev, davidsarah convinced me that two modes was appropriate because people who are uploading a file are not yet committed to the file being up, so it is better for them to abort in case of unsatisfying distribution, but people who are repairing an existing file are already committed to the file being out there, so it is better for them to do your best to make some improvement even in case of unsatisfying distribution.

comment:30 follow-up: Changed at 2010-10-31T04:30:42Z by zooko

  • Keywords news-needed added

Okay, I've now re-read this long, confusing ticket and I now agree that the patch Kevan already applied to make H be 0 during repair is correct. This means that repair processes always try to make progress (principle 2 from comment:26) instead of trying to conserve network bandwidth (principle 3 from comment:26), but upload processes (which aren't repairs) choose principle 3 instead of principle 2.

Also, yes, we really ought to fix #614 by defining healthy as "satisfying the servers-of-happiness level that my user has chosen". :-)

I don't think there's anything else to do but add a NEWS entry and then we can close this ticket. Does anyone else who is reading this agree?

comment:31 in reply to: ↑ 30 Changed at 2010-10-31T05:00:08Z by davidsarah

Replying to zooko:

Okay, I've now re-read this long, confusing ticket and I now agree that the patch Kevan already applied to make H be 0 during repair is correct. This means that repair processes always try to make progress (principle 2 from comment:26) instead of trying to conserve network bandwidth (principle 3 from comment:26), but upload processes (which aren't repairs) choose principle 3 instead of principle 2.

Also, yes, we really ought to fix #614 by defining healthy as "satisfying the servers-of-happiness level that my user has chosen". :-)

I don't think there's anything else to do but add a NEWS entry and then we can close this ticket. Does anyone else who is reading this agree?

Yes. There are still things we want to fix about repair (at least #614, #1124, and giving more complete information about the health of a file after repair), but let's address those for v1.9.0.

comment:32 Changed at 2010-10-31T06:14:06Z by david-sarah@…

In cb764da0edc2b161:

NEWS: entries for #1190 and #1212, and minor cleanups. refs #1190, #1212

comment:33 Changed at 2010-11-02T02:29:42Z by davidsarah

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

comment:34 in reply to: ↑ 14 Changed at 2011-01-05T03:53:21Z by davidsarah

Replying to davidsarah:

  • construct a maximum matching M : server -> share, of size |M|, for this file (preferring to include servers that are earlier on the permuted list when there is a choice).

and preferring to include servers that have least available space (especially those that are full), since that will allow uploads to succeed in more cases by placing new shares on servers that have available space.

comment:35 follow-up: Changed at 2011-06-08T13:52:15Z by zooko

  • Resolution fixed deleted
  • Status changed from closed to reopened

Diego "sickness" Righi is dissatisfied with this solution. He has 10 storage servers, and sets M=10 and H=10. His desire is that he never gets more than one share on one storage server. Current uploader does what he wants -- it never places more than one share on one storage server. But repairer does what he doesn't want -- if fewer than 10 storage servers are available then repairer uploads extra shares to some of the available servers.

To my way of thinking, uploading extra shares is making the file more available. For example if you have 8 servers with 1 share each and 1 server is 2 shares (and K=5), then if you lost the first five of your servers (each of which had 1 share) you could still recover your file from the remaining four servers. If instead you have 9 servers with one share each, K=5, and you lost the first five of your servers then the file would be lost.

So, now I'm going to stop here and ask sickness: does this cause you to change your mind so that now you want repairer to upload a second share to one of the existing servers in the case that there are only 9 servers available? Or do you still prefer that it should not do that?

comment:36 Changed at 2011-06-08T13:52:30Z by zooko

  • Owner set to sickness
  • Status changed from reopened to new

comment:37 Changed at 2011-06-08T14:05:05Z by gdt

I think sickness's desire for not having more than one share on a server blurs two things. One is having adequate redundancy, and a behavior of adding shares s.t. a server has two (in the 9 servers present case) helps. But, when the 10th server is back on line, if it doesn't have a share, then repair should consider the file unhealthy and place a share on the 10th server such that 10 servers have a distinct share. Further the lease on the extra share probably shouldn't be renewed.

If sickness also desires some form of storage efficiency, to avoid placing the 2nd share, then I think it's a misuse of servers-of-happiness and there should be some max-shares-per-server config, defaulting to infinite.

This all becomes difficult in the middle, when you have a 3/10 encoding and 3 or 4 servers. You want to set H to 3 or 4, but a share distribution of 7/1/1/1 isn't really ok - you want it to be more balanced. But I think we should figure out if this is a a reliability concern or an efficiency concern and treat them separately.

comment:38 Changed at 2011-06-08T14:05:54Z by gdt

Sorry, I didn't mean sickness's desire was blurry. I meant that on reading it, there are two issues possibly behind it, and we should be clear on which we are addressing and why.

comment:39 in reply to: ↑ 35 Changed at 2011-06-09T00:13:16Z by davidsarah

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

Replying to zooko:

Diego "sickness" Righi is dissatisfied with this solution.

The original problem in this ticket was that the repairer was using the default value for happiness, which was certainly wrong. Let's not overload the ticket; sickness' complaint is that the current repairer often places shares in a way that doesn't increase happiness, when another different placement of the same number of shares would have done so. That's covered by #1130.

comment:40 Changed at 2011-06-09T04:05:16Z by zooko

Hm, perhaps we should take this to tahoe-dev. Because I don't think that is sickness's complaint--I think his complaint is that it uploads more than one share to a server. I'll try to write a post for tahoe-dev.

comment:41 Changed at 2011-12-22T02:12:16Z by kmarkley86

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this ticket. I'm affected by the same fundamental problem, but by a different path. The fix identified earlier was to immutable/repairer.py, but I'm getting an error from immutable/upload.py.

Scenario: I'm using 2-of-4 encoding with shares.happy=4 on tahoe 1.8.1. From the CLI I do a tahoe check --repair on a file with shares {0, 2, 3} already existing on the grid but share 1 not existing, and I get an UploadUnhappinessError? complaining that "we were asked to place shares on at least 7" servers. There are only 4 servers on my grid -- hence my choice of shares.happy=4.

I observed that in immutable/upload.py, BaseUploadable? has a statement "default_encoding_param_happy = 7". I tried the experiment of changing this value to 4 (the shares.happy value in my tahoe.cfg) and then the repair succeeds without error.

So there must be a path through this code where the default_encoding_param_happy value is actually used instead of being overridden by the value in tahoe.cfg. (I think it smells a little that this object has defaults at all, instead of requiring the parameters to be provided.)

comment:42 Changed at 2011-12-29T00:25:37Z by zooko

  • Milestone changed from 1.8.1 to 1.9.1

Since this is a regression, I think we should consider trying to fix it for Tahoe-LAFS v1.9.1. Advice and help would be appreciated...

comment:43 Changed at 2011-12-29T22:37:14Z by davidsarah

  • Owner sickness deleted
  • Status changed from reopened to new

comment:44 Changed at 2011-12-29T22:43:30Z by davidsarah

  • Keywords reviewed news-needed removed

Please note that the scope of this ticket is just the fact that immutable/upload.py is incorrectly using default_encoding_param_happy = 7. As far as I know, we're not trying either to fix #1130 or to apply the refactoring/improvements to share placement in #1382 in Tahoe-LAFS 1.9.1.

comment:45 Changed at 2012-01-14T19:41:59Z by kevan

  • Owner set to kmarkley86

kmarkley86: A stack trace would help me fix this. Could you provide one?

comment:46 Changed at 2012-03-04T20:27:38Z by amontero

  • Cc amontero@… added

comment:47 Changed at 2012-03-31T23:15:25Z by davidsarah

  • Owner changed from kmarkley86 to warner
  • Priority changed from major to critical

The problem described in comment:41 is critical to fix for v1.9.2 (or 1.10.0 if we decide to call it that; the next release, anyway).

Last edited at 2012-03-31T23:16:17Z by davidsarah (previous) (diff)

comment:48 Changed at 2012-04-01T02:13:18Z by zooko

Oh wait, no hold on--this is a php script? No php on tahoe-lafs.org! Sorry.

comment:49 Changed at 2012-04-01T02:13:40Z by zooko

Wrong ticket. (should have been #1417)

comment:50 Changed at 2012-04-02T23:39:45Z by davidsarah

  • Keywords servers-of-happiness added

comment:51 Changed at 2012-06-12T16:50:42Z by davidsarah

  • Owner changed from warner to davidsarah
  • Status changed from new to assigned

comment:52 Changed at 2012-06-15T18:56:43Z by davidsarah

  • Summary changed from Repairing fails if less than 7 servers available to Upload (sometimes?) ignores shares.happy in tahoe.cfg

comment:53 Changed at 2012-06-19T03:20:20Z by david-sarah@…

In 196bd583b6c4959c:

(The changeset message doesn't reference this ticket)

comment:54 Changed at 2012-06-19T03:25:10Z by david-sarah <david-sarah@…>

In 196bd583b6c4959c:

Add assertions to make sure that set_default_encoding_parameters is always called, rather than using hardcoded 3/7/10 defaults. Also update affected tests. Note that this by itself cannot fix the bug mentioned in ticket:1212#comment:41, but it might make it easier to reproduce. refs #1212

comment:55 Changed at 2012-06-19T03:26:24Z by davidsarah

  • Owner changed from davidsarah to kmarkley86
  • Status changed from assigned to new

kmarkley86: can you try again to reproduce the problem in comment:41 using trunk?

comment:56 Changed at 2012-06-21T19:33:30Z by david-sarah@…

In 5521/1.9.2:

Add assertions to make sure that set_default_encoding_parameters is always called, rather than using hardcoded 3/7/10 defaults. Also update affected tests. Note that this by itself cannot fix the bug mentioned in ticket:1212#comment:41, but it might make it easier to reproduce. refs #1212

comment:57 Changed at 2012-06-21T19:43:26Z by david-sarah@…

In 5522/1.9.2:

Rollback last patch, which was pushed unintentionally to the 1.9.2 branch (Add assertions to make sure that set_default_encoding_parameters is always called...) refs #1212

comment:58 Changed at 2012-06-21T20:51:13Z by davidsarah

  • Milestone changed from 1.9.2 to 1.10.0

We decided to defer actually fixing the bug (if it still exists) to 1.10.

comment:59 Changed at 2012-07-16T16:33:55Z by david-sarah@…

In 5883/cloud-backend:

[rebased for cloud-backend] Add assertions to make sure that set_default_encoding_parameters is always called, rather than using hardcoded 3/7/10 defaults. Also update affected tests. Note that this by itself cannot fix the bug mentioned in ticket:1212#comment:41, but it might make it easier to reproduce. refs #1212

comment:60 Changed at 2012-09-04T16:27:09Z by zooko

Kyle: this ticket is blocked on you attempting to reproduce comment:41 using the new code, which has assertions that will let us learn more about the bug.

comment:61 Changed at 2012-10-26T02:50:29Z by davidsarah

  • Milestone changed from 1.10.0 to 1.8.1
  • Resolution set to fixed
  • Status changed from new to closed

Moved to #1830. The original problem was fixed in 1.8.1 I think. See #1130 and #1382 for other improvements to share placement and servers-of-happiness.

comment:62 Changed at 2012-10-26T02:54:56Z by davidsarah

  • Summary changed from Upload (sometimes?) ignores shares.happy in tahoe.cfg to Repair used default shares.happy

comment:63 follow-up: Changed at 2013-03-18T16:19:38Z by zooko

There was discussion of this issue on tahoe-dev: pipermail/tahoe-dev/2013-March/008091.html

comment:64 in reply to: ↑ 63 Changed at 2013-03-18T19:17:07Z by davidsarah

Replying to zooko:

There was discussion of this issue on tahoe-dev: pipermail/tahoe-dev/2013-March/008091.html

I'm sure that's not the same issue (nor is it the same issue as #1830).

Note: See TracTickets for help on using tickets.