On Wed, Aug 27, 2014 at 5:54 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 27 August 2014 19:42, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
> wrote:
> > On Tue, Aug 26, 2014 at 4:00 PM, C. Michael Pilato <cmpilato_at_collab.net>
> > wrote:
> >>
> >> On 08/26/2014 09:02 AM, Stefan Fuhrmann wrote:
> >
> >
> >>
> >> > Note that we
> >> > never include such headers in current Subversion code except
> >> > "fs-loader.h" and tests.
> >> >
> >> >
> >> > Would moving the declarations (2 structs, 10 functions)
> >> > to a new "include/private/svn_fs_fs_private.h" be sufficient
> >> > in your opinion?
> >>
> >> I should think that would be sufficient. But then, it wasn't my opinion
> >> that you solicited. :-)
> >
> >
> > Implemented in r1620909.
> >
> Stefan,
>
> This is completely wrong approach.
What problem are you trying to solve?
> Please revert immediately.
>
> Proper way is to implement three specific semi-private functions in
> libsvn_fs_fs for collecting stats, dumping and loading FS indexes and
> then use in svnfsfs.
>
Sounds like a good idea at the first glance, but it
* adds a large new API (instead of moving existing declarations)
* requires new code and various changes to existing code
* may require a separate library (which would defeat the whole purpose)
* can be done partly or entirely at any point in the future (no guarantees
made that could be broken)
So, there is no reason to do it just now and there are a few minor points
against it - considering that we will be branching 1.9 shortly. The
remainder
of this post only goes into more detail on some of the above.
This new set of API would still be fully private. Nothing for which I would
want to provide stability guarantees. If, at some point in the future, we
should provide more comprehensive and mature reporting and recovery
functionality, then there may be point to provide a stable API plus bindings
for integration into some admin environment.
Right now, this new API would still export the index data structure, i.e.
duplicate structures and #defines as well as add code to convert between
"internal" and "external" API. The current internal API based code is only
about 20 lines each for "load index" and "dump index". Everything else
is UI formatting and input parsing.
For the stats code, things are slightly different. Here, the progress and
results API would be quite complex (mainly the result data structures)
and involve lots of code churn (naming conventions, callbacks etc).
I'm also not fully convinced that such code should go into the FSFS core
library. One may make the point for the index access (it's recovery-ish)
but the stats code? That's data processing and I feel it should be kept
separate from the data provider module, i.e. in a libsvn_client-esque
library. My current opinion about that is not too strong. But I can see this
functionality grow in later releases, possibly adding entirely new features.
All that would bloat the core back-end.
-- Stefan^2.
Received on 2014-08-28 13:19:25 CEST