Update feature branch #6120
Merged
Update feature branch #6120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Vincent Liu <[email protected]>
There is no remote invocation of this function, so should be safe. Signed-off-by: Vincent Liu <[email protected]>
Rather than copy and copy', rename them into `copy_into_sr` and `copy_into_vdi`, which is what they actually do. Signed-off-by: Vincent Liu <[email protected]>
Also make the logging information more verbose. Signed-off-by: Vincent Liu <[email protected]>
- Introduce new `MigrateLocal` and `MigrateRemote` modules which contains the main implementations of the migration logic. - Move the actual exposed SMAPIv2 functions in one place towards the end of the file, rather than spreading across the entire file. These refactoring should all be statically verifiable by the compiler. Signed-off-by: Vincent Liu <[email protected]>
Signed-off-by: Vincent Liu <[email protected]>
Previously a task was constructed based on the log and tracing of a dbg of the type Debug_info.t, and then later on a dbg is constructed based on the previously constructued task. Instead of that, just convert the first dbg into a string and thread it through the call. Signed-off-by: Vincent Liu <[email protected]>
Signed-off-by: Colin James <[email protected]>
Now we got winbind as the Active Direcotry backend, the joined netbios name is persisted in smb.conf, thus can be compatible with hostname change This commit just remove the set_hostname_live limitation Signed-off-by: Lin Liu <[email protected]>
Reraise with the original stacktrace, this requires using the raw backtrace instead of the string one. Signed-off-by: Edwin Török <[email protected]>
…6116) Now we got winbind as the Active Direcotry backend, the joined netbios name is persisted in smb.conf, thus can be compatible with hostname change This commit just remove the set_hostname_live limitation
Reraise with the original stacktrace, this requires using the raw backtrace instead of the string one. I had this change sitting in a branch since around Apr 25 2024, although the branch itself might need more testing, this is a simple fix that we should probably get in sooner.
Flame graphs indicate that, under load created by parallel "xe vm-list" commands, the DB action get_record is hit often. This function constructs an API-level record by marshalling an association list that maps field names to unmarshalled string values. To do this, it serially queries all the field names using List.assoc. This has rather large cost in doing lexicographical string comparisons (caml_compare on string keys). To avoid this, regardless of record size, we preprocess the association lists __regular_fields and __set_refs into a (string, string) Hashtbl.t and query that to construct each record field. Signed-off-by: Colin James <[email protected]>
Rewrite the API.assocer function to internally construct a hash table from an association list. Then, shadow its usage in relevant places. Signed-off-by: Colin James <[email protected]>
Speed up "find" operation on Schema Tables and Database. Previously, these would use List.find which involves costly comparisons on string keys. Now, we use a hash table, keyed by names, and use that to identify columns and tables. Signed-off-by: Colin James <[email protected]>
Sync the metadata with upstream, which is not updated Signed-off-by: Pau Ruiz Safont <[email protected]>
The function copy is being dropped because it's to_string, without labeled arguments Signed-off-by: Pau Ruiz Safont <[email protected]>
Sync the metadata with upstream, which is not updated This fixes the CI issue due to cstruct version in xs-opam being incompatible with the one requested by the opam metadata in this repository
Flame graphs indicate that, under load created by parallel "xe vm-list"
commands, the DB action get_record is hit often. This function
constructs an API-level record by marshalling an association list that
maps field names to unmarshalled string values. To do this, it serially
queries all the field names using `List.assoc`. This has rather large
cost in doing lexicographical string comparisons (`caml_compare` on
string keys).
To avoid this, regardless of record size, we preprocess the association
lists `__regular_fields` and `__set_refs` into a (string, string)
Hashtbl.t and query that to construct each record field.
---
This benefit of this change is most notable for large records (such as
`VM` and `pool`). The cost of the previously generated code, which does
a bunch of serial `List.assoc` calls, incurs the quadratic cost of list
traversal (compounded by the costly lexicographical comparison of
strings during the search).
To produce measurements, I sampled xapi under a load of 500 consecutive
`xe vm-list` invocations (using the same sampling rate with `perf`) on a
host with a single VM.
Without the change, the `get_record` done internally by `xe vm-list`
makes up for ~33.59% of the samples (33.59% = 1,592,782,255 samples).
With the change, `get_record` accounts for ~7.56 of the samples (of
which there are substantially fewer collected: 7.56% = 264,948,239). So
the number of samples for `get_record` has dropped from 1,592,782,255 to
264,948,239 (assuming `perf`'s sampling is reliable).
You can see the visual difference in the flame graphs:
Before:
After:
Of course, this is benefit as measured in aggregate (`perf` sampling),
so quite a fast and loose comparison. In practice, the `xe vm-list`
stress test goes from 7.4s to 6.2s (as `get_record` makes up only a
small part of the work done for a single `xe vm-list`).
The Storage migration code is too messy to be maintainable, making the developing experience quite bad. This is the first attempt to make it less spaghetti. I am intending to do this in several stages. This PR is just for those that are mostly "safe" operations that can probably be checked statically by the compiler. More risky changes on the way... Best reviewed by commit.
lindig
approved these changes
Nov 18, 2024
mg12
approved these changes
Nov 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.