You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2014/08/26 17:19:25 UTC
Implement major FSFS performance related changes in the experimental
FSX format
I propose to design and implement all major performance related
changes of the current FSFS format in the experimental FSX format.
I mean features exactly like revprop caching and log addressing.
The best possible option for such features is to be implemented and
released as a part of experimental FSX. Then, when we will be really
sure that everything is fine and it works for the wide number of users,
we can selectively port (not merge) some of the features into the FSFS.
We have successfully followed this approach in the past (FSFS itself
and ra_serf) and currently I do not see any reasons to change this
approach for features I'm talking about. Moreover, we have discussed
this on Berlin 2013 hackathon [1]:
[[[
stefan2 expressed that while he is confident that FSFSv7 is solid code,
it's also quite critical and could easily take a year or more to fully
stabilize. Attendees felt that perhaps it would be best to introduce FSFSv7
as a new, experimental fs-type. Stefan said he had been thinking
about the same thing himself, even considering a different name for
his implementation.
]]]
At this point I'm '-1' to:
1) release the improved version of revprop caching to the FSFS format [2]
2) release the log addressing feature in the FSFSv7 format
The current implementation of revprop caching and log-addressing features
should be reverted from trunk and moved to FSX.
[1] http://wiki.apache.org/subversion/Berlin2013#FSFSv7_Branch_Reintegration
[2] http://svn.apache.org/repos/asf/subversion/branches/revprop-caching-ng
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Branko Čibej <br...@wandisco.com>.
Thanks, Mike, for explaining this better than I could. As usual, I might
add!
-- Brane
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 30 September 2014 20:01, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 26 September 2014 20:34, Stefan Fuhrmann
> <st...@wandisco.com> wrote:
>> On Fri, Sep 26, 2014 at 5:07 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
[...]
>> Not being Mike, here is my opinion anyway: I'm +1 on your proposal.
>> In fact, I had planned to call for exactly that vote at some point mid-Oct
>> (so I could complete work on the other bits e.g. svnfsfs before that).
>>
>> Due to the size of the features (plural), I think we should extend the
>> voting period to at least 2 weeks. This will give people who are new
>> to the code a chance to actually review it. I don't think there is any
>> point in rushing a decision at this stage of 1.9 "slippage".
>>
> Stefan,
>
> It seems that you started to change fsfs code actively, so I postpone
> creation of the formal vote until the code and data format (!) will
> be "stabilized" again.
>
> Please let me know when you finish.
Hi Stefan,
Could you please confirm that the log-addressing code and data format
are stable now and ready for review and vote.
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 September 2014 20:34, Stefan Fuhrmann
<st...@wandisco.com> wrote:
> On Fri, Sep 26, 2014 at 5:07 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>> Mike,
>>
>> > At a minimum, the community needs to establish that this feature and its
>> > side-effects are understood by more than just the one smart guy that
>> > wrote
>> > it and the other smart guy who isn't a fan. Then, those who understand
>> > the
>> > feature and its side-effects need to publicly weigh in on both the value
>> > and the timing (1.9, FSFS rather than FSX, etc.) of the change.
>>
>> > How do you manage this discussion in the simplest way possible? Call
>> > for
>> > a formal vote on removing the feature, asking that the extreme +1/-1
>> > votes
>> > be presented only by folks who both understand the feature and have
>> > reviewed
>> > the code. (Seems only fair to allow the status quo to remain the
>> > default
>> > action.) Give the vote at least 72 weekday hours to allow time for code
>> > review, and then put this topic behind you/us and move on.
>>
>> Since no one objected to this approach, I assume that there is a lazy
>> consensus and I'm going to start a formal vote regarding the
>> log-addressing feature early next week.
>>
>> I think it would be fair to call a 'Consensus Approval' vote [1,2] for
>> leaving
>> the log-addressing feature in the trunk. In other words, it will be
>> required
>> at least three binding '+1' votes (and no vetos) to leave the code in
>> trunk.
>>
>> It will be also assumed that:
>> a) +1 votes could be presented only by folks who both understand
>> the log-addressing feature and have reviewed the code.
>> b) a concrete technical justification showing why the change is bad
>> (allows data to be corrupted, negatively affects performance, etc. )
>> should be provided for a '-1' vote.
>>
>> Effectively, this vote will be similar to our 3-vote policy for branches
>> but made a little later.
>>
>> What do you think about this plan for vote?
>
>
> Hi Ivan,
>
> Not being Mike, here is my opinion anyway: I'm +1 on your proposal.
> In fact, I had planned to call for exactly that vote at some point mid-Oct
> (so I could complete work on the other bits e.g. svnfsfs before that).
>
> Due to the size of the features (plural), I think we should extend the
> voting period to at least 2 weeks. This will give people who are new
> to the code a chance to actually review it. I don't think there is any
> point in rushing a decision at this stage of 1.9 "slippage".
>
Stefan,
It seems that you started to change fsfs code actively, so I postpone
creation of the formal vote until the code and data format (!) will
be "stabilized" again.
Please let me know when you finish.
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Tue, Sep 30, 2014 at 19:30:06 +0200:
> And I hope I won't be left alone when having to fix FSFSv7 corruption
> should it ever occur in the wild ;-)
I see the wink, but I'll answer seriously: if that's a concern, we might
want to review the 'structure' file and code comments, to see if they
describe all they need to so that any developer (not just stefan2) can
become familiar with the code to the degree that they can fix any bugs
therein.
In other words, we should review the documentation, separately from
reviewing the code.
For example, the 'structure' file usually documents how we *do* things
--- but we don't always document *why* we do them one way or another
(design choices), what aspects may be changed (flexibility), and what
changes should be avoided (e.g., for real-world reasons that our test
suite doesn't catch) (pitfalls).
Daniel
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Sep 30, 2014 at 05:45:48PM +0100, Julian Foad wrote:
> ***
> Who has reviewed this or intends to do so? And for what aspects -- overall
> design, compatibility/upgrade issues, threading issues, style, ...?
> ***
I am not planning to review the FSFSv7 changes because the changes
are too large and complex for me to digest in a reasonable amount
of time. From my point of view they have accumulated to a giant code
dump developed by a single developer (much like the original FSFS code,
as Greg once told me in Berlin). They are the result of much of Stefan's
time and effort, which is awesome, but which he also developed mostly
alone, which is bad for collaboration if it happens over such a long
period of time. I wish Ivan and Stefan had developed these changes
together, in lock step, from the beginning. I hope we can learn from
this in the future and adapt our way of working accordingly, somehow.
I hope to have made a small but valuable contribution towards FSFSv7's
quality by making sure there is no 1980's atoi()-style code in it that
never heard of integer overflows. But beyond that I see no way of
catching up. I came to this conclusion after glancing over the files
that Ivan pointed out earlier we should read.
I'd rather invest my time elsewhere and hope the community will settle
on something that is good enough. Given that this discussion is still
on-going and obviously considered important, I still have hope that
this will be the case.
And I hope I won't be left alone when having to fix FSFSv7 corruption
should it ever occur in the wild ;-)
Re: Implement major FSFS performance related changes in the experimental FSX format
Posted by Julian Foad <ju...@btopenworld.com>.
C. Michael Pilato wrote:
> On 09/26/2014 12:34 PM, Stefan Fuhrmann wrote:
>> Not being Mike, here is my opinion anyway: I'm +1 on your proposal.
>
> I'm late to the party, but this all sounds great.
I need to decide for myself: should *I* volunteer to review it?
As a member of our community of Subversion developers, I will soon be expected to support and maintain a new release of our code base. To play my part, I want to be satisfied that what we produce is progressing in a reasonably sane direction.
Sometimes I review other people's code; more often I trust that somebody else will do so. And normally that works out OK -- somebody does review most changes, probably, and any issues get addressed, and that's enough for me. On this occasion, if we need more volunteers to review it in order to contribute to the community's shared understanding then I will consider doing so. But there is a small problem: I have very little awareness of who reviews what, so it's hard to know if that investment of effort is necessary.
So I want to ask, please:
***
Who has reviewed this or intends to do so? And for what aspects -- overall
design, compatibility/upgrade issues, threading issues, style, ...?
***
Once a few people have reviewed the code, there will be enough people qualified to vote on it, but at the same time there will also be enough people to have a reasoned discussion and I would hope and expect they will settle any issues by consensus. In the end, ideally, there should be no need for a vote. It seems to me that the real objective behind calling for a vote in this case is to call for review.
- Julian
Re: Implement major FSFS performance related changes in the experimental
FSX format
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/26/2014 12:34 PM, Stefan Fuhrmann wrote:
> On Fri, Sep 26, 2014 at 5:07 PM, Ivan Zhakov <ivan@visualsvn.com
> <ma...@visualsvn.com>> wrote:
>
> I think it would be fair to call a 'Consensus Approval' vote [1,2]
> for leaving
> the log-addressing feature in the trunk. In other words, it will
> be required
> at least three binding '+1' votes (and no vetos) to leave the code
> in trunk.
>
> It will be also assumed that:
> a) +1 votes could be presented only by folks who both understand
> the log-addressing feature and have reviewed the code.
> b) a concrete technical justification showing why the change is bad
> (allows data to be corrupted, negatively affects performance,
> etc. )
> should be provided for a '-1' vote.
>
> Effectively, this vote will be similar to our 3-vote policy for
> branches
> but made a little later.
>
> What do you think about this plan for vote?
>
>
> Hi Ivan,
>
> Not being Mike, here is my opinion anyway: I'm +1 on your proposal.
> In fact, I had planned to call for exactly that vote at some point mid-Oct
> (so I could complete work on the other bits e.g. svnfsfs before that).
>
> Due to the size of the features (plural), I think we should extend the
> voting period to at least 2 weeks. This will give people who are new
> to the code a chance to actually review it. I don't think there is any
> point in rushing a decision at this stage of 1.9 "slippage".
>
> -- Stefan^2.
I'm late to the party, but this all sounds great.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Sep 26, 2014 at 5:07 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> Mike,
>
> > At a minimum, the community needs to establish that this feature and its
> > side-effects are understood by more than just the one smart guy that
> wrote
> > it and the other smart guy who isn't a fan. Then, those who understand
> the
> > feature and its side-effects need to publicly weigh in on both the value
> > and the timing (1.9, FSFS rather than FSX, etc.) of the change.
>
> > How do you manage this discussion in the simplest way possible? Call for
> > a formal vote on removing the feature, asking that the extreme +1/-1
> votes
> > be presented only by folks who both understand the feature and have
> reviewed
> > the code. (Seems only fair to allow the status quo to remain the default
> > action.) Give the vote at least 72 weekday hours to allow time for code
> > review, and then put this topic behind you/us and move on.
>
> Since no one objected to this approach, I assume that there is a lazy
> consensus and I'm going to start a formal vote regarding the
> log-addressing feature early next week.
>
> I think it would be fair to call a 'Consensus Approval' vote [1,2] for
> leaving
> the log-addressing feature in the trunk. In other words, it will be
> required
> at least three binding '+1' votes (and no vetos) to leave the code in
> trunk.
>
> It will be also assumed that:
> a) +1 votes could be presented only by folks who both understand
> the log-addressing feature and have reviewed the code.
> b) a concrete technical justification showing why the change is bad
> (allows data to be corrupted, negatively affects performance, etc. )
> should be provided for a '-1' vote.
>
> Effectively, this vote will be similar to our 3-vote policy for branches
> but made a little later.
>
> What do you think about this plan for vote?
>
Hi Ivan,
Not being Mike, here is my opinion anyway: I'm +1 on your proposal.
In fact, I had planned to call for exactly that vote at some point mid-Oct
(so I could complete work on the other bits e.g. svnfsfs before that).
Due to the size of the features (plural), I think we should extend the
voting period to at least 2 weeks. This will give people who are new
to the code a chance to actually review it. I don't think there is any
point in rushing a decision at this stage of 1.9 "slippage".
-- Stefan^2.
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Ivan Zhakov <iv...@visualsvn.com>.
Mike,
> At a minimum, the community needs to establish that this feature and its
> side-effects are understood by more than just the one smart guy that wrote
> it and the other smart guy who isn't a fan. Then, those who understand the
> feature and its side-effects need to publicly weigh in on both the value
> and the timing (1.9, FSFS rather than FSX, etc.) of the change.
> How do you manage this discussion in the simplest way possible? Call for
> a formal vote on removing the feature, asking that the extreme +1/-1 votes
> be presented only by folks who both understand the feature and have reviewed
> the code. (Seems only fair to allow the status quo to remain the default
> action.) Give the vote at least 72 weekday hours to allow time for code
> review, and then put this topic behind you/us and move on.
Since no one objected to this approach, I assume that there is a lazy
consensus and I'm going to start a formal vote regarding the
log-addressing feature early next week.
I think it would be fair to call a 'Consensus Approval' vote [1,2] for leaving
the log-addressing feature in the trunk. In other words, it will be required
at least three binding '+1' votes (and no vetos) to leave the code in trunk.
It will be also assumed that:
a) +1 votes could be presented only by folks who both understand
the log-addressing feature and have reviewed the code.
b) a concrete technical justification showing why the change is bad
(allows data to be corrupted, negatively affects performance, etc. )
should be provided for a '-1' vote.
Effectively, this vote will be similar to our 3-vote policy for branches
but made a little later.
What do you think about this plan for vote?
[1] http://www.apache.org/foundation/voting.html
[2] http://www.apache.org/foundation/glossary.html#ConsensusApproval
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the experimental
FSX format
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/18/2014 08:26 AM, Ivan Zhakov wrote:
> On 11 September 2014 18:03, C. Michael Pilato <cm...@collab.net> wrote:
> Mike,
>
>> Ivan: As a reviewer of the code in question, there's really no way
>> you could have established (for yourself) a legitimate objection
>> about the "general quality" of the code without first spotting
>> some specific things that bothered you. Of course, if while
>> reviewing the code you spotted so many such specific things
>> that you lost count or couldn't keep track, that general
>> concern would emerge.
> This is a case when the code contains so many complications and
> unclear design and implementation issues that nobody can effectively
> review it.
Thanks for explaining, Ivan.
It sounds to me, then, as if you are a bit more concerned with code
complexity, bus factor, and the detrimental performance that result from
this feature than with "code quality" per se. The nomenclature matters,
because an accusation of poor /quality/ in code will inevitably come
across as a slant at the developer. (Code of the highest quality can
still be complicated and result in decreased performance of the
application.) Now, most of us would probably agree on whether a given
piece of code is written with quality. But depending on our own comfort
with the nuances of the language, our knowledge of the code context,
etc., we might all disagree on whether it's "too complicated". The bus
factor thing is similarly difficult to measure. Moreover, I recall
seeing conflicting bits of information addressing the performance
aspects of the feature. It would seem we are without a gold standard of
sorts for these more fuzzy determinations of the value of the change.
Unfortunately, this all makes your case for removing the feature a
harder one to press.
It's unfortunate that there are essentially only two technical voices in
this discussion and that they stand in opposition to each other. Brane,
I, and others have weighed in a meta level, but have offered essentially
nothing that would help to resolve the /technical/ standoff (that I
recall seeing, at least). Having reviewed the mailing list thread, I
did see an effective +1 from Philip on merging the branch to trunk[1]
(which does not require him to prove that he groks the change). Daniel
also explicitly +1'd the statement that getting the feature into an
alpha release would be a better way to test it[2], but that of course is
not a value judgment of the change itself. Your suggestion for a vote
on the branch being merged wasn't out of line given the amount of
contention over the issue, but the suggestion never stuck -- that's just
life in a community of equal voices.
My opinion, FWIW: I know nothing about the technical details of this
change. But I am confident that it is unwise to move forward with
things in the state they are today, where "state" includes the code
itself and what we know about its bus factor. At a minimum, the
community needs to establish that this feature and its side-effects are
understood by more than just the one smart guy that wrote it and the
other smart guy who isn't a fan. Then, those who understand the feature
and its side-effects need to publicly weigh in on both the value and the
timing (1.9, FSFS rather than FSX, etc.) of the change. Sure, there's
nothing in Ye Olde HACKING that requires this stuff, but these are the
kinds of concessions that communities of volunteers make -- for the good
of the community and the project -- when the typical patterns are
failing. And if someone wants to go on record as opposing such a
simple, simple exercise that could go a long way towards community
harmony ... well, so be it. In my mind, it's their own credibility at
stake. How do you manage this discussion in the simplest way possible?
Call for a formal vote on removing the feature, asking that the extreme
+1/-1 votes be presented only by folks who both understand the feature
and have reviewed the code. (Seems only fair to allow the status quo to
remain the default action.) Give the vote at least 72 weekday hours to
allow time for code review, and then put this topic behind you/us and
move on.
My $0.015.
-- Mike
[1] http://svn.haxx.se/dev/archive-2013-11/0191.shtml
[2] http://svn.haxx.se/dev/archive-2013-11/0267.shtml
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 11 September 2014 18:03, C. Michael Pilato <cm...@collab.net> wrote:
Mike,
> Ivan: As a reviewer of the code in question, there's really no way
> you could have established (for yourself) a legitimate objection
> about the "general quality" of the code without first spotting
> some specific things that bothered you. Of course, if while
> reviewing the code you spotted so many such specific things
> that you lost count or couldn't keep track, that general
> concern would emerge.
This is a case when the code contains so many complications and
unclear design and implementation issues that nobody can effectively
review it. To show this I'll just retell the story.
1. We have discussed on Berlin 2013 hackathon that the log-addressing
feature should be implemented in an experimental fs-type [1].
[[
stefan2 expressed that while he is confident that FSFSv7 is solid code,
it's also quite critical and could easily take a year or more to fully
stabilize. Attendees felt that perhaps it would be best to introduce FSFSv7
as a new, experimental fs-type. Stefan said he had been thinking
about the same thing himself, even considering a different name for
his implementation.
]]
Then the plan was changed but there are no any footprints on the
dev list about the reasons to change the direction.
2. The log-addressing feature was implemented in the branch. The branch
was proposed for review in [2].
At this point I volunteered to review the code. My initial feeling
was that I'm about '-0.5' to merge this branch. The reasons were
the following:
1) I do not think that the value of the log-adressing feature worth
the code burden it produces.
2) I think that code is over-complicated and too error-prone.
I raised my concerns in that thread.
3. I suggested to use a 3-vote policy for this branch but this suggestion
was not actually implemented.
The formal vote thread was not created. There is a single informal '+1'
to merge this branch. There are also statements that two other committers
have started the review but there are no any footprints in the dev list
that they have even finished it.
Then the log-addressing feature was pushed to the trunk without any notice
in the dev list.
I'm not a big fan of the 3-vote policy for branches but I do not understand
why this policy was not actually applied for this particular branch.
4. On the Berlin 2014 hackathon serious design flaw and significant
performance degradation have found in the log-addressing feature.
I'm talking about storing indexes in separate files. This is a big issue
and it is not a surprise that this issue was not found before merging
to trunk. I treat this as a sign that nobody other that an sole author
understands what actually happens in the code.
5. Accidentally, significant new issues were detected in the revprop-caching
feature (see [3], [4], [5] etc).
It does matter because the revprop-caching was implemented with the same
level of overconfidence as the discussed log-addressing feature. We have
got enormous number of issues in the revprop-caching feature.
The frightening thing is that the already released feature was
disabled in the trunk and we are going to disable it in the next
patch release. This is an exceptional case. I do not remember
other cases when the released feature becomes disabled in the patch
release.
Once again. This is an exceptional case and I do not see any reasons
to be carefree and say something like 'bugs always happen'.
6. At this point I have switched to '-1' about the log-addressing feature.
It's obvious that we will be unable to easily disable the log-addressing
feature as we have done for the revprop-caching. And we can get many
repositories corrupted. And the format can be changed (as it happened
at the Berlin hackathon when the feature is supposed to be properly
reviewed and already merged to trunk). And we will be obliged to support
repositories with the current log-addressing format forever or ask users
to dump repositories using svn 1.9 and load using svn 1.10.
These are the main reasons why I'm vetoing this feature from being
released in its current state. Also I still think that there are
no performance benefits for the majority of users.
This is the full story. I personally think that this is far enough
for the veto.
The current question is the following: is it a legitimate veto or not?
I think it is fully legitimate. The topic is discussed for the long
time but nobody wants to put an additional '+1' for the discussed
code (formally or informally). I think this happens because of the fact
that the code is too big and too overcomplicated. So we should not
release it in order to prevent the possible disaster.
Do we really need to find the particular bugs in this situation?
What will happen if data corruption or similar issues will be found?
How we will be able to proof that there are no more other
critical issues (when the found ones will be fixed)?
Once again, I know that bugs always happen. But we are talking
about significant *repository* format change. We should
be very sure that everything is fine and the new format is stable
(with all the implementation details).
[1] http://wiki.apache.org/subversion/Berlin2013#FSFSv7_Branch_Reintegration
[2] dev@s.a.o thread "Log-addressing branch ready for review"
http://svn.haxx.se/dev/archive-2013-08/0364.shtml
[3] dev@s.a.o thread "[RFC] Revision property caching and named atomics"
http://svn.haxx.se/dev/archive-2014-08/0235.shtml
[4] dev@s.a.o thread "named_atomic tests failures on Windows"
http://svn.haxx.se/dev/archive-2014-09/0048.shtml
[5] http://svn.haxx.se/dev/archive-2014-08/0007.shtml
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the experimental
FSX format
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/10/2014 05:37 PM, Branko Čibej wrote:
>
>
> > I have technical objections related to the general quality of the
> > new code. The best way to find out more is to review this code by
> > yourself!
>
> No. It's your veto and therefore the burden of evidence is on you.
> "General quality of code" is the same kind of nonsense argument as
> "code churn." By that criterion, we'd have never released 1.0.
>
> Throwing diffs my way that you know damn well I can produce myself is
> just avoiding the issue. You're obviously able to point out concrete
> problems in other parts of the code, so I can't understand why that's
> so hard in the case of log addressing. Maybe you see issues there that
> I missed, so please, once again, take the time to point them out. You
> may even help the author learn something.
>
> Sure, it's a lot of work. So is writing this mail on a phone while
> lying in hospital, but no-one else is likely to do it for me.
>
Branko: Actually, I was going to write it for you, but you beat me to
it. :-) Ivan, I honestly don't want to "pile on" here, but Branko is
right.
Ivan: As a reviewer of the code in question, there's really no way you
could have established (for yourself) a legitimate objection about the
"general quality" of the code without first spotting some /specific/
things that bothered you. Of course, if while reviewing the code you
spotted so many such specific things that you lost count or couldn't
keep track, that general concern would emerge. That's all completely
natural, and it would be silly for anyone to claim that your concern
about a bit of code's quality is a bogus feeling. Unfortunately,
"general concerns" aren't portable. If you wish to adequately
communicate your concern to anyone else in hopes that they, too, agree
that the code needs to be fixed, you're going to have to stop talking
about the effect of your review (general concern) and return to the
fundamental causes that created that concern -- specific code defects.
Depending on how fresh in your mind the code is, this may mean
completely reviewing the code again for yourself, making shareable notes
about the problematic code bits one by one as you do so. That is, you
need to offer a typically line-by-line, block-by-block code review
yourself. I'm not claiming that this will be enjoyable, but it is what
is required in a community of volunteers.
-- C-Mike
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Branko Čibej <br...@wandisco.com>.
On 10 Sep 2014 16:40, "Ivan Zhakov" <iv...@visualsvn.com> wrote:
>
> Brane,
>
> On 9 September 2014 19:15, Branko Čibej <br...@wandisco.com> wrote:
> > I'm not in the mood to split hairs right now (those who need to
> > know why). I'll just say that I haven't had time to
> > review the revprop cache branch, so obviously I can neither
> > approve nor veto it. The general approach was
> > discussed at the hackathon in Sheffield, and I did agree with that.
>
> Could you please stop putting pressure on other developers when
> you haven't actually read the code discussed?
You vetoed some change in the code. I asked you to produce concrete
arguments for your veto. If you call that pressure, you're on the wrong
list.
> I have technical objections related to the general quality of the
> new code. The best way to find out more is to review this code by
> yourself!
No. It's your veto and therefore the burden of evidence is on you. "General
quality of code" is the same kind of nonsense argument as "code churn." By
that criterion, we'd have never released 1.0.
Throwing diffs my way that you know damn well I can produce myself is just
avoiding the issue. You're obviously able to point out concrete problems in
other parts of the code, so I can't understand why that's so hard in the
case of log addressing. Maybe you see issues there that I missed, so
please, once again, take the time to point them out. You may even help the
author learn something.
Sure, it's a lot of work. So is writing this mail on a phone while lying in
hospital, but no-one else is likely to do it for me.
> Once again. It's totally counterproductive to discuss issues related
> to the quality and over-complication of code if one of the parties
> doesn't know anything about the particular code
And once again you're making assumptions about what I do or don't know ...
and totally missing the point, which I hope I explained clearly enough
above.
-- Brane
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Ivan Zhakov <iv...@visualsvn.com>.
Brane,
On 9 September 2014 19:15, Branko Čibej <br...@wandisco.com> wrote:
> I'm not in the mood to split hairs right now (those who need to
> know why). I'll just say that I haven't had time to
> review the revprop cache branch, so obviously I can neither
> approve nor veto it. The general approach was
> discussed at the hackathon in Sheffield, and I did agree with that.
Could you please stop putting pressure on other developers when
you haven't actually read the code discussed?
The code is not always as good as you can imagine or expect. And
it is not always as good as you could be told about it.
> Ivan, since you called me out about techical grounds, I'll
> just repeat what I already said: if you have specific objections,
> point them out. Saying "I object!" is just not good enough.
I have technical objections related to the general quality of the
new code. The best way to find out more is to review this code by
yourself! You can use the 'remove-log-addressing' branch to make
the corresponding diff. To help you, I'm attaching patch for
'remove-log-addressing' branch with current log-addressing
implementation. libsvn_fs_fs\index.c is my favorite one :)
Please review the code and put your '+1' if you personally
agree with the overall design and all the significant implementation
details of log-addressing feature.
Once again. It's totally counterproductive to discuss issues related
to the quality and over-complication of code if one of the parties
doesn't know anything about the particular code (excluding the
knowledge about the 'general approach' that is expected to be
followed in the code).
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Branko Čibej <br...@wandisco.com>.
I'm not in the mood to split hairs right now (those who need to know why).
I'll just say that I haven't had time to review the revprop cache branch,
so obviously I can neither approve nor veto it. The general approach was
discussed at the hackathon in Sheffield, and I did agree with that.
Ivan, since you called me out about techical grounds, I'll just repeat what
I already said: if you have specific objections, point them out. Saying "I
object!" is just not good enough.
-- Brane
On 8 Sep 2014 18:24, "Ivan Zhakov" <iv...@visualsvn.com> wrote:
> > (FWIW, maintaining the remove-log-addressing branch doesn't count, and
> > frankly I have a hard time imagining how that branch could be less buggy
> > than trunk, given that it's received far less attention and testing.)
> As you should know, current FSFS code on trunk is like this:
> if (log_addressing_enabled(rev))
> {
> do_one();
> }
> else
> {
> do_else();
> }
>
> And this covers *all* cases, because FSFS code supports reading and
> *writing* all previous FSFS formats. I've just removed do_one() calls on
> remove-log-addressing branch. How this could be more buggy?? Please
> provide more details about this point.
>
> > You must have forgotten the history of ra_serf: almost nobody used it
> > until we made it the default and only option, and so we only started
> > getting useful feedback after the 1.8.0 release.
>
> We release features not just to get a 'useful feedback' from the users.
> There was a lot of work to prepare ra_serf to be the default option. I
> guess
> we would have a different kind of feedback if this work have not been done.
>
> >> Moreover, we have discussed
> >> this on Berlin 2013 hackathon [1]:
> >> [[[
> >> stefan2 expressed that while he is confident that FSFSv7 is solid code,
> >> it's also quite critical and could easily take a year or more to fully
> >> stabilize. Attendees felt that perhaps it would be best to introduce
> FSFSv7
> >> as a new, experimental fs-type. Stefan said he had been thinking
> >> about the same thing himself, even considering a different name for
> >> his implementation.
> >> ]]]
> > Yup, we did; more than a year ago. A lot has changed since then.
> What exactly have changed and where it was discussed?
>
> > I'll even go as far as to suspect that you're not aware there's no
> revprop
> > caching on trunk right now.
>
> Should I suspect that you are saying this to make it seem that I don't have
> any technical ground behind my position?
>
> As I said elsethread, I routinely read everyone's commits to Subversion
> codebase (with different level of attention, of course). Moreover, both
> problems that caused to disable revprop caching on trunk were initially
> found by two of my colleagues. So, despite the fact that I've been on
> vacation when r1619774 was committed, I obviously know the story.
>
> Also, I do not see yours '+1' or other kind of positive feedback regarding
> the revprop-caching-ng branch.
>
> Have you got a chance to review the revprop-caching-ng branch?
>
> Do you agree with all the significant technical solutions adopted in
> this branch?
>
>
> --
> Ivan Zhakov
>
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Sep 8, 2014 at 6:24 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>
> >> Moreover, we have discussed
> >> this on Berlin 2013 hackathon [1]:
> >> [[[
> >> stefan2 expressed that while he is confident that FSFSv7 is solid code,
> >> it's also quite critical and could easily take a year or more to fully
> >> stabilize. Attendees felt that perhaps it would be best to introduce
> FSFSv7
> >> as a new, experimental fs-type. Stefan said he had been thinking
> >> about the same thing himself, even considering a different name for
> >> his implementation.
> >> ]]]
> > Yup, we did; more than a year ago. A lot has changed since then.
> What exactly have changed and where it was discussed?
>
One obvious thing would be that what was called "FSFSv7" back then
is now called "FSX". FSFS f7 today contains a simplified backport of
the logical addressing in FSX.
Completing FSX and stabilizing it will still take years - last not least
because the final shape of it is unknown. Without the split, we might
by now have a first rough implementation of all the (now FSX) features
that I had in mind last spring. It would still be far from stable, so
having *one* similar feature implemented and fully tested in FSFS
also helps the FSX development. More importantly, it gives users
sizeable benefits today.
-- Stefan^2.
Re: Implement major FSFS performance related changes in the
experimental FSX format
Posted by Ivan Zhakov <iv...@visualsvn.com>.
> (FWIW, maintaining the remove-log-addressing branch doesn't count, and
> frankly I have a hard time imagining how that branch could be less buggy
> than trunk, given that it's received far less attention and testing.)
As you should know, current FSFS code on trunk is like this:
if (log_addressing_enabled(rev))
{
do_one();
}
else
{
do_else();
}
And this covers *all* cases, because FSFS code supports reading and
*writing* all previous FSFS formats. I've just removed do_one() calls on
remove-log-addressing branch. How this could be more buggy?? Please
provide more details about this point.
> You must have forgotten the history of ra_serf: almost nobody used it
> until we made it the default and only option, and so we only started
> getting useful feedback after the 1.8.0 release.
We release features not just to get a 'useful feedback' from the users.
There was a lot of work to prepare ra_serf to be the default option. I guess
we would have a different kind of feedback if this work have not been done.
>> Moreover, we have discussed
>> this on Berlin 2013 hackathon [1]:
>> [[[
>> stefan2 expressed that while he is confident that FSFSv7 is solid code,
>> it's also quite critical and could easily take a year or more to fully
>> stabilize. Attendees felt that perhaps it would be best to introduce FSFSv7
>> as a new, experimental fs-type. Stefan said he had been thinking
>> about the same thing himself, even considering a different name for
>> his implementation.
>> ]]]
> Yup, we did; more than a year ago. A lot has changed since then.
What exactly have changed and where it was discussed?
> I'll even go as far as to suspect that you're not aware there's no revprop
> caching on trunk right now.
Should I suspect that you are saying this to make it seem that I don't have
any technical ground behind my position?
As I said elsethread, I routinely read everyone's commits to Subversion
codebase (with different level of attention, of course). Moreover, both
problems that caused to disable revprop caching on trunk were initially
found by two of my colleagues. So, despite the fact that I've been on
vacation when r1619774 was committed, I obviously know the story.
Also, I do not see yours '+1' or other kind of positive feedback regarding
the revprop-caching-ng branch.
Have you got a chance to review the revprop-caching-ng branch?
Do you agree with all the significant technical solutions adopted in
this branch?
--
Ivan Zhakov
Re: Implement major FSFS performance related changes in the experimental
FSX format
Posted by Branko Čibej <br...@wandisco.com>.
On 26.08.2014 17:19, Ivan Zhakov wrote:
> I propose to design and implement all major performance related
> changes of the current FSFS format in the experimental FSX format.
You do realise, I hope, that it's quite likely that FSX will end up
having very little in common with FSFS. It might not even be a
monolithic filesystem; remember we talked about splitting the current FS
into two modules, metadata and content; if we do that in FSX, we'll have
FSX-meta and FSX-content and neither will be close to FSFS.
Your proposal therefore boils down to requiring someone to maintain
/two/ experimental codebases, FSFSv7 and FSX. And I didn't see you
volunteer for either of those tasks ...
(FWIW, maintaining the remove-log-addressing branch doesn't count, and
frankly I have a hard time imagining how that branch could be less buggy
than trunk, given that it's received far less attention and testing.)
> I mean features exactly like revprop caching and log addressing.
>
> The best possible option for such features is to be implemented and
> released as a part of experimental FSX. Then, when we will be really
> sure that everything is fine and it works for the wide number of users,
> we can selectively port (not merge) some of the features into the FSFS.
See above. At some point, merges between FSX and FSFS will become about
as reasonable as merges between BDB and FSFS.
> We have successfully followed this approach in the past (FSFS itself
> and ra_serf) and currently I do not see any reasons to change this
> approach for features I'm talking about.
You must have forgotten the history of ra_serf: almost nobody used it
until we made it the default and only option, and so we only started
getting useful feedback after the 1.8.0 release.
> Moreover, we have discussed
> this on Berlin 2013 hackathon [1]:
> [[[
> stefan2 expressed that while he is confident that FSFSv7 is solid code,
> it's also quite critical and could easily take a year or more to fully
> stabilize. Attendees felt that perhaps it would be best to introduce FSFSv7
> as a new, experimental fs-type. Stefan said he had been thinking
> about the same thing himself, even considering a different name for
> his implementation.
> ]]]
Yup, we did; more than a year ago. A lot has changed since then.
> At this point I'm '-1' to:
> 1) release the improved version of revprop caching to the FSFS format [2]
You did not even review the branch, or if you did, you didn't write
anything to the dev@ list about it. I'll even go as far as to suspect
that you're not aware there's no revprop caching on trunk right now. So
what's your argument for the veto? The intent of the changes on the
branch is to fix an existing serious bug in released code.
> 2) release the log addressing feature in the FSFSv7 format
I've asked you about a dozen times now to point out actual flaws in the
log addressing implementation. The performance regression you found has
been fixed, and other than that, I don't recall you having any other
concrete issue than the fact that there are actual code changes involved.
Furthermore, there are more new features in trunk FSFS than just log
addressing, and that is arguably the most exhaustively tested of those
features. Why do you keep talking only about log addressing and not, for
example, non-blocking packing? That's a performance enhancement, too.
-- Brane