Skip to content

Commit

Permalink
ui, server: update cluster settings to include last update time
Browse files Browse the repository at this point in the history
Resolves cockroachdb#76626

Previously it was difficult to determine from the cluster
settings page which settings have been altered.
This patch updates the cluster settings endpoint to include a
last updated field, indicating when the setting was last altered,
which allows the settings page to include that column in the
table and is pre-sorted by this value.

Release justification: Fairly minor changes for QoL upgrade
Release note: None
  • Loading branch information
Santamaura committed Jun 1, 2022
1 parent 96a2f03 commit 331c147
Show file tree
Hide file tree
Showing 6 changed files with 519 additions and 368 deletions.
1 change: 1 addition & 0 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -5019,6 +5019,7 @@ SettingsResponse is the response to SettingsRequest.
| type | [string](#cockroach.server.serverpb.SettingsResponse-string) | | | [reserved](#support-status) |
| description | [string](#cockroach.server.serverpb.SettingsResponse-string) | | | [reserved](#support-status) |
| public | [bool](#cockroach.server.serverpb.SettingsResponse-bool) | | | [reserved](#support-status) |
| last_updated | [google.protobuf.Timestamp](#cockroach.server.serverpb.SettingsResponse-google.protobuf.Timestamp) | | | [reserved](#support-status) |



Expand Down
31 changes: 30 additions & 1 deletion pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -1686,21 +1686,50 @@ func (s *adminServer) Settings(
lookupPurpose = settings.LookupForReporting
}

// Read the system.settings table to determine the settings for which we have
// explicitly set values -- the in-memory SV has the set and default values
// flattened for quick reads, but we'd only need the non-defaults for comparison.
alteredSettings := make(map[string]*time.Time)
if it, err := s.server.sqlServer.internalExecutor.QueryIteratorEx(
ctx, "read-setting", nil, /* txn */
sessiondata.InternalExecutorOverride{User: security.RootUserName()},
`SELECT name, "lastUpdated" FROM system.settings`,
); err != nil {
log.Warningf(ctx, "failed to read settings: %s", err)
} else {
var ok bool
for ok, err = it.Next(ctx); ok; ok, err = it.Next(ctx) {
row := it.Cur()
name := string(tree.MustBeDString(row[0]))
lastUpdated := row[1].(*tree.DTimestamp)
alteredSettings[name] = &lastUpdated.Time
}
if err != nil {
// No need to clear AlteredSettings map since we only make best
// effort to populate it.
log.Warningf(ctx, "failed to read settings: %s", err)
}
}

resp := serverpb.SettingsResponse{KeyValues: make(map[string]serverpb.SettingsResponse_Value)}
for _, k := range keys {
v, ok := settings.Lookup(k, lookupPurpose)
if !ok {
continue
}
var altered *time.Time
if val, ok := alteredSettings[k]; ok {
altered = val
}
resp.KeyValues[k] = serverpb.SettingsResponse_Value{
Type: v.Typ(),
// Note: v.String() redacts the values if the purpose is not "LocalAccess".
Value: v.String(&s.server.st.SV),
Description: v.Description(),
Public: v.Visibility() == settings.Public,
LastUpdated: altered,
}
}

return &resp, nil
}

Expand Down
16 changes: 15 additions & 1 deletion pkg/server/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ func TestAdminAPISettings(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer s.Stopper().Stop(context.Background())

// Any bool that defaults to true will work here.
Expand Down Expand Up @@ -1097,6 +1097,20 @@ func TestAdminAPISettings(t *testing.T) {
if typ != v.Type {
t.Errorf("%s: expected type %s, got %s", k, typ, v.Type)
}
if v.LastUpdated != nil {
db := sqlutils.MakeSQLRunner(conn)
q := makeSQLQuery()
q.Append(`SELECT name, "lastUpdated" FROM system.settings WHERE name=$`, k)
rows := db.Query(
t,
q.String(),
q.QueryArguments()...,
)
defer rows.Close()
if rows.Next() == false {
t.Errorf("missing sql row for %s", k)
}
}
}

t.Run("all", func(t *testing.T) {
Expand Down
Loading

0 comments on commit 331c147

Please sign in to comment.