Fix data races in dashboard response construction
Dashboard responses had data races because multiple goroutines were reading and modifying dashboards before sending them out on the wire. This patch introduces immutability in the construction of the response, so that each goroutine is working with its own set of dashboardResponse structs.pull/1797/head
parent
e5331dc536
commit
25059c0591
|
@ -30,27 +30,34 @@ func newCellResponses(dID chronograf.DashboardID, dcells []chronograf.DashboardC
|
|||
base := "/chronograf/v1/dashboards"
|
||||
cells := make([]dashboardCellResponse, len(dcells))
|
||||
for i, cell := range dcells {
|
||||
if len(cell.Queries) == 0 {
|
||||
cell.Queries = make([]chronograf.DashboardQuery, 0)
|
||||
}
|
||||
newCell := chronograf.DashboardCell{}
|
||||
|
||||
newCell.Queries = make([]chronograf.DashboardQuery, len(cell.Queries))
|
||||
copy(newCell.Queries, cell.Queries)
|
||||
|
||||
// ensure x, y, and y2 axes always returned
|
||||
labels := []string{"x", "y", "y2"}
|
||||
if cell.Axes == nil {
|
||||
cell.Axes = make(map[string]chronograf.Axis, len(labels))
|
||||
}
|
||||
newCell.Axes = make(map[string]chronograf.Axis, len(labels))
|
||||
|
||||
newCell.X = cell.X
|
||||
newCell.Y = cell.Y
|
||||
newCell.W = cell.W
|
||||
newCell.H = cell.H
|
||||
newCell.Name = cell.Name
|
||||
newCell.ID = cell.ID
|
||||
|
||||
for _, lbl := range labels {
|
||||
_, found := cell.Axes[lbl]
|
||||
if !found {
|
||||
cell.Axes[lbl] = chronograf.Axis{
|
||||
if axis, found := cell.Axes[lbl]; !found {
|
||||
newCell.Axes[lbl] = chronograf.Axis{
|
||||
Bounds: []string{},
|
||||
}
|
||||
} else {
|
||||
newCell.Axes[lbl] = axis
|
||||
}
|
||||
}
|
||||
|
||||
cells[i] = dashboardCellResponse{
|
||||
DashboardCell: cell,
|
||||
DashboardCell: newCell,
|
||||
Links: dashboardCellLinks{
|
||||
Self: fmt.Sprintf("%s/%d/cells/%s", base, dID, cell.ID),
|
||||
},
|
||||
|
|
|
@ -28,20 +28,19 @@ type getDashboardsResponse struct {
|
|||
|
||||
func newDashboardResponse(d chronograf.Dashboard) *dashboardResponse {
|
||||
base := "/chronograf/v1/dashboards"
|
||||
DashboardDefaults(&d)
|
||||
AddQueryConfigs(&d)
|
||||
cells := newCellResponses(d.ID, d.Cells)
|
||||
templates := newTemplateResponses(d.ID, d.Templates)
|
||||
dd := AddQueryConfigs(DashboardDefaults(d))
|
||||
cells := newCellResponses(dd.ID, dd.Cells)
|
||||
templates := newTemplateResponses(dd.ID, dd.Templates)
|
||||
|
||||
return &dashboardResponse{
|
||||
ID: d.ID,
|
||||
Name: d.Name,
|
||||
ID: dd.ID,
|
||||
Name: dd.Name,
|
||||
Cells: cells,
|
||||
Templates: templates,
|
||||
Links: dashboardLinks{
|
||||
Self: fmt.Sprintf("%s/%d", base, d.ID),
|
||||
Cells: fmt.Sprintf("%s/%d/cells", base, d.ID),
|
||||
Templates: fmt.Sprintf("%s/%d/templates", base, d.ID),
|
||||
Self: fmt.Sprintf("%s/%d", base, dd.ID),
|
||||
Cells: fmt.Sprintf("%s/%d/cells", base, dd.ID),
|
||||
Templates: fmt.Sprintf("%s/%d/templates", base, dd.ID),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
@ -229,24 +228,28 @@ func ValidDashboardRequest(d *chronograf.Dashboard) error {
|
|||
return err
|
||||
}
|
||||
}
|
||||
DashboardDefaults(d)
|
||||
(*d) = DashboardDefaults(*d)
|
||||
return nil
|
||||
}
|
||||
|
||||
// DashboardDefaults updates the dashboard with the default values
|
||||
// if none are specified
|
||||
func DashboardDefaults(d *chronograf.Dashboard) {
|
||||
func DashboardDefaults(d chronograf.Dashboard) (newDash chronograf.Dashboard) {
|
||||
newDash.Cells = make([]chronograf.DashboardCell, len(d.Cells))
|
||||
for i, c := range d.Cells {
|
||||
CorrectWidthHeight(&c)
|
||||
d.Cells[i] = c
|
||||
newDash.Cells[i] = c
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// AddQueryConfigs updates all the celsl in the dashboard to have query config
|
||||
// objects corresponding to their influxql queries.
|
||||
func AddQueryConfigs(d *chronograf.Dashboard) {
|
||||
func AddQueryConfigs(d chronograf.Dashboard) (newDash chronograf.Dashboard) {
|
||||
newDash.Cells = make([]chronograf.DashboardCell, len(d.Cells))
|
||||
for i, c := range d.Cells {
|
||||
AddQueryConfig(&c)
|
||||
d.Cells[i] = c
|
||||
newDash.Cells[i] = c
|
||||
}
|
||||
return
|
||||
}
|
||||
|
|
|
@ -128,7 +128,7 @@ func TestDashboardDefaults(t *testing.T) {
|
|||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
if DashboardDefaults(&tt.d); !reflect.DeepEqual(tt.d, tt.want) {
|
||||
if actual := DashboardDefaults(tt.d); !reflect.DeepEqual(actual, tt.want) {
|
||||
t.Errorf("%q. DashboardDefaults() = %v, want %v", tt.name, tt.d, tt.want)
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue