Skip to content

Database: document, re-add a test and use a value type rather than strings #2018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 21 commits into from
Nov 14, 2014
Merged

Database: document, re-add a test and use a value type rather than strings #2018

merged 21 commits into from
Nov 14, 2014

Conversation

@djs55
Copy link
Contributor

@djs55 djs55 commented Nov 13, 2014

We

  • fix up the database interface to make it look more uniform and more like the stdlib
  • use a value type rather than strings, to avoid excess s-expression marshalling and unmarshalling
  • untangle a unit test from a client/server test and run this from the build
  • add missing licensing headers

David Scott added 20 commits November 12, 2014 11:24
Database: add a 'type' to each column
5099b9a 
The types are:
 - String: most fields
 - Set: all Set(_)
 - Pairs: all Map(_,_)

This allows a database implementation to avoid serialising sets and maps
to strings all the time.

This patch also adds 'with sexp' to the schema so it can be easily read
and written. Eventually we will be able to separate the datamodel and
the schema and have the database load the schema at runtime.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: rename MAP2 to MAP, Map2 to Make and extract a common signa…
ca4588f 
…ture

Names like "MAP2" beg the question, "what happened to MAP1?"

This patch also factorises the ROW, TABLE, TABLESET signatures into a
shared common portion, with specific extensions on top.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: replace the raw 'int64' with a 'Time.t' (= Generation.t)
0369d63 
This means that
- if you read the type, you realise this isn't just any old int64
- we are closer to making it fully abstract

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: every function which takes Time.t -> Time.t has the third T…
e6ebd30 
…ime.t

Some of the functions omitted the third Time.t (the deleted time), because
the object could not have been deleted yet. Elsewhere we have the convention
that a live object has deleted = 0L.

This patch makes all the Time.t -> Time.t (-> Time.t) functions consistent.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: every function which takes 3 Time.ts now takes Stat.t
5c6786f 
The 3 Time.t's are like a file "stat":
  - a create time
  - a last-modified time
  - a deletion time (since the events are historical)

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: fold_over_recent takes a function with the same signature a…
e832605 
…s fold

Previously fold and fold_over_recent used a slightly different permutation
of arguments in the function being folded over the data. The OCaml stdlib
convention is to have a function (key -> value -> 'a -> 'a) so we follow
this.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: split Table.fold_over_recent into fold_over_recent and fold…
8ca4d22 
…_over_deleted

This allows fold_over_recent to have the same signature as the other
functions with the same name. Tracking of deleted references (without values)
is specific to the Table.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: fold_over_recent have the same signature, so put in the com…
d7291d5 
…mon definition

Now that all the common MAP instances have the same fold_over_recent signature,
we can share the definition and avoid people playing "spot the difference"
with a complicated type.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: remove the errf from fold_over_{recent,deleted} as no-one w…
1e4119b 
…as using it

To define an error case from a fold you can either
- turn the accumulator into a result type and return `Error
- throw an exception (... and document this and guarantee to catch it)

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: add missing license headers
be115bd 
Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: start adding ocamldoc to the interface
4773f63 
Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: remove Table.find_exn
bc25135 
This was confusing because we already had {Row,Table,TableSet}.find
which threw exceptions. The only difference was that Table.find_exn
would throw a DBCache exception rather than Not_found.

We revert all callers to Table.find, and expect them (in the DB_ACCESS
layer) to convert the Not_found into the DB_ACCESS-specific exception
themselves.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: rename 'update_generation' to 'touch' and simplify the sign…
1fd3f51 
…ature

The 'update_generation' function was defined as
- always bump the generation count (in contract to 'update')
- also transform one binding to another value

In practice all users were using it only to bump the generation count
like "touch"ing a file. Therefore we remove the transform ability and
rename the function "touch".

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: use the same 'remove' function signature everywhere
18ddcf3 
This allows us to put the 'remove' function into the MAP signature
for consistency. Now Row, Table, TableSet all have a remove which
takes a Time.t which allows tracking of delete events. We only
use these in the Table to track deleted Rows, but maybe in future
we could track deleted Tables or fields later?

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: remove unused function 'rows'
875efa2 
This was a trivial use of fold anyway.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: add ocamldoc for the 'fold_over_deleted' function
8248d0f 
Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: remove some signatures duplicated from the .mli
c7678d5 
Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: add a 'Value.t' to the schema
177c20c 
The schema can use this value for describing Column.ts, in particular
- the 'empty' value
- the optional 'default' value

The Value.t is still being marshalled to a string for storage in the
database.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Database: fill the database with Schema.Value.t rather than strings
277c852 
In particular this means that sets and maps are not automatically serialised
to strings via the 'add_to' and 'remove_from' code. However the wire protocol
still uses s-expressions so the more slaves, the more s-expressions you'll see.
Database: re-add a test for the database library
74fd925 
The library test can be run easily from the build.

The remainder of 'database_test' is really a 'database server test'
which doesn't currently work.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
@jonludlam
Copy link
Contributor

jonludlam commented Nov 13, 2014

Niiiice

@johnelse
Copy link
Contributor

johnelse commented Nov 14, 2014

Does the unit test exit 1 if it fails? I only ask because I keep getting bitten by this - you might have to do something like this:

https://github.com/xenserver/gpumon/blob/master/test/test_main.ml#L9-10

@djs55
Copy link
Contributor Author

djs55 commented Nov 14, 2014

Hm, I added a "failwith" in the test and got this:

djs@st30:~/djs55/xen-api$ ./ocaml/database/database_test.opt 
E
==============================================================================
Error: tar:0:many to many

Failure("foo")
------------------------------------------------------------------------------
Ran: 1 tests in: 0.00 seconds.
FAILED: Cases: 1 Tried: 1 Errors: 1 Failures: 0 Skip:  0 Todo: 0 Timeouts: 0.
djs@st30:~/djs55/xen-api$ echo $?
0

Grrr

Database: unit test now exits with non-zero on error (woo)
54a3698 
Thanks to @johnelse for pointing this out.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
@djs55
Copy link
Contributor Author

djs55 commented Nov 14, 2014

@jonludlam is happy to merge this now that the xen-4.5 changes have been built by the CI

djs55 added a commit that referenced this pull request Nov 14, 2014
@djs55
Merge pull request #2018 from djs55/forupstream/database-schema
0471acd 
Database: document, re-add a test and use a value type rather than strings
@djs55 djs55 merged commit 0471acd into xapi-project:master Nov 14, 2014
@djs55 djs55 deleted the forupstream/database-schema branch November 14, 2014 20:46
@jonludlam
Copy link
Contributor

jonludlam commented Nov 15, 2014

Just for clarity, the Xen 4.5 changes mentioned above are only internal changes to be able to use Xen 4.5, not any actual changes to the source that may be required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@djs55 @jonludlam @johnelse @robhoes