Skip to content
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.

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

CP-49140: prepare for database optimizations #6181

Draft
wants to merge 2 commits into
base: feature/perf
Choose a base branch
from

Conversation

edwintorok
Copy link
Contributor

No description provided.

Currently all of these are strings, but it may change:
* table
* db_ref
* field_name
* field
* uuid

This enables changing the `field` type from `string`
and avoiding costly serialization/deserialization when not needed.

No functional change.

Signed-off-by: Edwin Török <[email protected]>
sexp_of is used when saving the schema, but of_sexp is unused.

This enables a future commit to store the deserialized value as a pair value,
and deserialization (unmarshaling) requires knowing the schema,
whereas serialization (marshaling) does not.
Thus we couldn't implement a generic `t_of_sexp` function, but with this change
we won't have to.

No functional change.
@psafont
Copy link
Member

psafont commented Dec 13, 2024

I'm tempted to suggest that this PR should be opened against master, it seems quite straightforward

Comment on lines +88 to 90
val read_field : Db_ref.t -> table -> db_ref -> field_name -> field
(** [read_field context tbl ref fld] returns the value of field [fld]
in row [ref] in table [tbl] *)
Copy link
Contributor

@contificate contificate Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be careful here, the doc comment is actually incorrect.

The correct order is:

val read_field : Db_ref.t -> table -> field_name -> db_ref -> field

See a corresponding DB action:

    and get_enabled ~__context ~self =
      let self = DM_to_String.ref_Observer self in
      let __t = Context.database_of __context in
      let module DB = (val (Xapi_database.Db_cache.get __t) : Xapi_database.Db_interface.DB_ACCESS) in
      String_to_DM.bool (DB.read_field __t "Observer" "enabled" self)

Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mosty introduces type aliases; a preparation to change these types in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants