[ovs-dev] [PATCH 1/5] ovsdb-idl: Compound Indexes Design Document

[Ben Pfaff]

On Wed, Mar 09, 2016 at 12:01:53AM +0000, Rodriguez Betancourt, Esteban wrote:
> This is the pull request of the complete change: https://github.com/openvswitch/ovs/pull/111
> 
> ---
> From d7d82c305a610fdb9a81427aa5483e8c6a071687 Mon Sep 17 00:00:00 2001
> From: Esteban Rodriguez Betancourt <estebarb at hpe.com>
> Date: Fri, 26 Feb 2016 17:23:28 -0600
> Subject: [PATCH 1/5] ovsdb-idl: Compound Indexes Design Document
> 
> In the work made in our projects, it was found the need to have a faster
> access to the rows contained in tables in the replica, as well to have
> the possibility to loop over a subset of rows that meet some specified
> criteria.
> Those needs lead us to design and implement a functionality that
> satisfies those requirements, so an implementation of special indexes were
> done.
> In order to keep the OVSDB server implementation unmodified and avoid
> extra load of processing, the indexes are created as part of the IDL.
> The indexes are created as part of the initialization of the replica request
> and are maintained automatically when there are changes in the replica.
> 
> This document explains the design rationale of the compound indexes feature.
> 
> Signed-off-by: Javier Albornoz <javier.albornoz at hpe.com>
> Signed-off-by: Esteban Rodriguez Betancourt <estebarb at hpe.com>
> Signed-off-by: Jorge Arturo Sauma Vargas <jorge.sauma at hpe.com>
> Co-authored-by: Javier Albornoz <javier.albornoz at hpe.com>
> Co-authored-by: Esteban Rodriguez Betancourt <estebarb at hpe.com>
> Co-authored-by: Jorge Arturo Sauma Vargas <jorge.sauma at hpe.com>

Thanks for working on this!  I agree that this is a useful feature.  It
is probably overdue.

Thank you for writing a design document.  I think that the design
document should probably be under the Documentation directory, though,
because it is not something that a new user will look for.  We probably
need to start moving more of our documents into that directory.

One point that I would like the design document to make near the
beginning is that this is all about the OVSDB client.  At first, I
thought that there would be some change to the OVSDB server or protocol;
I'm happy to see that this is all about improving the C client library.

As I continue reading through the document, I see that this is
mentioned:
+It's important to mention that all changes will be done in the IDL. There are no
+changes to the OVSDB server or the OVSDB engine.

Perhaps this could just be moved closer to the beginning.

To make the Markdown format properly (e.g. in "make dist-docs" output),
the diagram needs to be indented two more spaces and tabs converted to
spaces, and the "# " on the first line of the file should be deleted.

The README describes the compound indexes in terms of changes being made
to the IDL.  This is the correct point of view for a commit log, but for
a documentation file inside a program's own repository it is confusing
to anyone who comes to the project after the feature is added.  It means
that someone new to the project has to understand the whole history of a
feature to understand how to use it.  Thus, I would prefer that the
design document describes the feature and how to use it, rather than
describe the changes from what was here before.  This may not be a big
change to the text, actually, but I consider the shift in point of view
important anyway.

Probably, the exact definitions of the structs should not go in the
documentation, for at least a few reasons.  First, the structs will
probably change over time and thus get out of sync with the
documentation.  Second, the reader does not need the exact definitions
to understand how the feature works (they are supposed to be opaque
anyway) and can easily go find them if he or she is interested.

Similarly, I would not put the exact prototypes and per-function
documentation in the design doc.  The examples, on the other hand, in
"Index Creation Example" and "Indexes Querying", seem valuable, although
I do wonder whether they should go in comments in the code instead or as
well.

I'll continue to look at the later patches.  Thanks again!