-
Notifications
You must be signed in to change notification settings - Fork 55
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
Replace old location and latency management with cleaner and easier to use design #137
Open
meling
wants to merge
28
commits into
master
Choose a base branch
from
config-and-latency-cleanup
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
Previously, we used json format in this generator. This is the start of replacing all latency configuration with one pre-generated latency matrix that is easy to use via the internal/latencies package.
This refactors the latencygen tool to save the time.Duration latencies (ms) in a 2D slice indexed by the cities in the cities slice.
Adds a comment /* Index City name */ in front of each line in the latency_matrix.go. Adds a comment for the 2D slice. Divides the round-trip values read from the csv file by 2 to get the one-way latency.
This adds two new methods Latencies() takes a location name (string) and returns a slice of latencies (time.Duration) indexed by the positions of the locations in the locations slice. LocationName() simply returns the location name of the given index position.
This adds the LatenciesFrom constructor function based on the provided locations slice; the returned LatencyMatrix is indexed in both directions following the provided location slice's indexes. This also provides a LatencyMatrix.LatencyID(a, b hotstuff.ID) to get the latency between a pair of nodes based on their ID.
We should not need to access the locations slice externally.
We should not need to access the global latencies matrix externally.
We should not access the global latencies matrix externally as hotstuff.IDs. Use the replacement LatencyMatrix.LatencyID() method.
This renames the LatencyCity function that operates on the full set of locations. This is now called Latency(a, b string). This also renames LatencyID method on the LatencyMatrix to just Latency(a, b hotstuff.ID) as well. It should not be possible to confuse the two; one is a method on LatencyMatrix and takes a pair of hotstuff.ID values, whereas the other is a function and takes a pair of string location values. This also improves documentation, explaining that providing invalid input to the Latency functions will cause a panic.
This renames latencies to latency to follow Go's best practices. This also adds DO NOT EDIT comment for the latency_matrix.go file Renames locations to allLocations and latencies to allLatencies to avoid stealing good names elsewhere and better explain that these represent all locations and latencies. Renames latency.LatencyMatrix to latency.Matrix and latency.Latency to latency.Between to avoid stutter. Simiarly, latency.LatenciesFrom has been renamed to latency.MatrixFrom(locations) Matrix. This also adds a latency.Location(id hotstuff.ID) function; this is mainly useful for debugging purposes and ease of use. This also adds an Enabled() bool method since I don't want to use a pointer for the latency.Matrix.
This replaces the old multi-map-based location and latencies lookups that were a bit awkward to work with. This removes the existing map[string]map[string]time.Duration{} latencies using the data in the internal/latency package. The new latency package provides the lm.Latency(a, b) to get the latency between two hotstuff.IDs. We now only need to fill the Locations []string in the ReplicaOpts to pass the set of locations around.
This also adds tests for Enabled().
This moves hotstuff.DefaultLocation to the latency package, adds checks for invalid hotstuff.IDs and default location.
This replaces logging output every 100 message with a time-based approach that only logs the number of commands output every second. This avoids filling up the logs with useless messages, but still allows reasonable progress to be identified.
Since we don't need to start and stop the timer, there is no reason to not use the simpler time.Sleep() call instead of this: timer1 := time.NewTimer(delay) <-timer1.C
Calling this on the latency matrix providing nodes a and b, it will delay (sleep for the duration) of the latency between a and b.
This moves the ParseCSVLatencies and GenerateGoLatencyMatrix funcs to the cli package to make the functions available to other parts of the codebase, if necessary. Right now, it is only used by the latencygen tool, but the new cli.LatencyMatrix() could be used to convert a parsed CSV file to a latency matrix at runtime instead of using the pre-compiled code generated by GenerateGoLatencyMatrix.
This allows to switch the latency matrix to another supported csv file; currently aws.csv and wonderproxy.csv. To support other csv files, they can be added to the latencies folder, and then you can run the generator like this: latencygen -file other_latencies.csv
Previously, we only had a latencies.csv file; we now can switch between two different ones: aws.csv and wonderproxy.csv. This commit fixes it so that the correct label is included in the generated latency_matrix.go file. The latency.SourceFile variable can be used by experiments to log the CSV source file used to run the experiment.
WithLatencies is a more representative name for this option.
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.
This PR makes the location and latencies management easier to manage and use for ourselves.
This is in preparation for an improved configuration file design and orchestration design.
This work will arrive in a follow up PR.