Skip to content

Commit

Permalink
[#1510] Use float for RPS representation
Browse files Browse the repository at this point in the history
  • Loading branch information
msiodelski committed Sep 26, 2024
1 parent 80bba3a commit c4a394d
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 41 deletions.
6 changes: 4 additions & 2 deletions api/dhcp-defs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,11 @@
monitored:
type: boolean
rps1:
type: integer
type: number
format: float
rps2:
type: integer
type: number
format: float
haEnabled:
type: boolean
haOverview:
Expand Down
19 changes: 2 additions & 17 deletions backend/server/apps/kea/rps.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@ import (
storkutil "isc.org/stork/util"
)

// Precision of the calculated RPS. The value of 100
// means two fractional digits. The calculated RPS is
// multiplied by the precision and sent to the UI as
// an integer. The received value is then divided by
// the precision to obtain a value with two fractional
// digits.
const RpsPrecision = 100

// Periodic Puller that generates RPS interval data.
type RpsWorker struct {
db *pg.DB
Expand Down Expand Up @@ -307,9 +299,7 @@ func (rpsWorker *RpsWorker) updateKeaDaemonRpsStats(daemon *dbmodel.Daemon) erro
}

// Calculate the RPS for the first row in a set of RpsIntervals.
// The calculated RPS is an actual RPS multiplied by RpsPrecision.
// It allows for displaying fractional RPS in the UI.
func calculateRps(totals []*dbmodel.RpsInterval) int64 {
func calculateRps(totals []*dbmodel.RpsInterval) float32 {
if len(totals) == 0 {
return 0
}
Expand All @@ -320,13 +310,8 @@ func calculateRps(totals []*dbmodel.RpsInterval) int64 {
return 0
}

// If the RPS is below the precision, let's return the minimal value.
if RpsPrecision*responses <= duration {
return 1
}

// Return the rate.
return RpsPrecision * responses / duration
return float32(responses) / float32(duration)
}

// Returns the statistic value and sample time from a given row within a
Expand Down
18 changes: 7 additions & 11 deletions backend/server/apps/kea/rps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ func TestRpsWorkerPullRps(t *testing.T) {
require.EqualValues(t, 7, interval.Responses)

// Verify daemon RPS stat values are as expected.
checkDaemonRpsStats(t, db, 1, 200, 200)
checkDaemonRpsStats(t, db, 2, 300, 300)
checkDaemonRpsStats(t, db, 1, 2, 2)
checkDaemonRpsStats(t, db, 2, 3, 3)
}

// Verifies that getting stat values that are less than or equal to the previous
Expand Down Expand Up @@ -269,7 +269,7 @@ func rpsTestAddMachine(t *testing.T, db *dbops.PgDB, dhcp4Active bool, dhcp6Acti
}

// Verifies RPS values for both intervals for a given daemon.
func checkDaemonRpsStats(t *testing.T, db *dbops.PgDB, keaDaemonID int64, interval1 int64, interval2 int64) {
func checkDaemonRpsStats(t *testing.T, db *dbops.PgDB, keaDaemonID int64, interval1 float32, interval2 float32) {
daemon := &dbmodel.KeaDHCPDaemon{}
err := db.Model(daemon).
Where("kea_daemon_id = ?", keaDaemonID).
Expand All @@ -282,15 +282,15 @@ func checkDaemonRpsStats(t *testing.T, db *dbops.PgDB, keaDaemonID int64, interv
// be rounded down. Let's add a margin of 1 to these checks. Without it, the test
// results were unstable.
require.Condition(t, func() bool {
return daemon.Stats.RPS1 == interval1 || daemon.Stats.RPS1 == interval1-1
return daemon.Stats.RPS1 <= interval1 || daemon.Stats.RPS1 >= interval1-1
}, "RPS1: %d, interval1: %d", daemon.Stats.RPS1, interval1)
require.Condition(t, func() bool {
return daemon.Stats.RPS2 == interval2 || daemon.Stats.RPS2 == interval2-1
return daemon.Stats.RPS2 <= interval2 || daemon.Stats.RPS2 >= interval2-1
}, "RPS2: %d, interval2: %d", daemon.Stats.RPS2, interval2)
}

// Calculate the RPS from an array of RpsIntervals.
func getExpectedRps(rpsIntervals []*dbmodel.RpsInterval, endIdx int) int64 {
func getExpectedRps(rpsIntervals []*dbmodel.RpsInterval, endIdx int) float32 {
var responses int64
var duration int64

Expand All @@ -303,11 +303,7 @@ func getExpectedRps(rpsIntervals []*dbmodel.RpsInterval, endIdx int) int64 {
return 0
}

if RpsPrecision*responses < duration {
return 1
}

return RpsPrecision * responses / duration
return float32(responses) / float32(duration)
}

// Marshall a given json response to a DHCP4 command and pass that into Response4Handler.
Expand Down
4 changes: 2 additions & 2 deletions backend/server/database/model/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ const (
// A structure reflecting Kea DHCP stats for daemon. It is stored
// as a JSONB value in SQL and unmarshaled in this structure.
type KeaDHCPDaemonStats struct {
RPS1 int64 `pg:"rps1"`
RPS2 int64 `pg:"rps2"`
RPS1 float32 `pg:"rps1"`
RPS2 float32 `pg:"rps2"`
}

// A structure holding Kea DHCP specific information about a daemon. It
Expand Down
73 changes: 73 additions & 0 deletions backend/server/database/model/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1024,3 +1024,76 @@ func TestDeleteKeaDaemonConfigHashes(t *testing.T) {
require.Equal(t, "", daemons[0].KeaDaemon.ConfigHash)
require.Equal(t, "", daemons[1].KeaDaemon.ConfigHash)
}

// Test RPS statistics inserted as integers can be later
// fetched as floats. It tests the data type change in the
// RPS stats from int to float32.
func TestGetRpsStatsAsFloats(t *testing.T) {
db, _, teardown := dbtest.SetupDatabaseTestCase(t)
defer teardown()

m := &Machine{
ID: 0,
Address: "localhost",
AgentPort: 8080,
}
err := AddMachine(db, m)
require.NoError(t, err)
require.NotZero(t, m.ID)

// add app but without machine, error should be raised
app := &App{
ID: 0,
MachineID: m.ID,
Type: AppTypeKea,
Daemons: []*Daemon{
NewKeaDaemon(DaemonNameDHCPv4, true),
},
}
_, err = AddApp(db, app)
require.NoError(t, err)
require.NotNil(t, app)
require.Len(t, app.Daemons, 1)
daemon := app.Daemons[0]
require.NotZero(t, daemon.ID)

// This is the statistics structure we used to have for the
// RPS. The RPS were stored as int.
type testKeaDHCPDaemonStats struct {
RPS1 int `pg:"rps1"`
RPS2 int `pg:"rps2"`
}

// This structure ties the old RPS formats to the Kea DHCP daemon.
type testKeaDHCPDaemon struct {
tableName struct{} `pg:"kea_dhcp_daemon"` //nolint:unused
ID int64 `pg:",pk"`
KeaDaemonID int64
Stats testKeaDHCPDaemonStats
}

// Update the Kea daemon with RPS values stored as int.
keaDaemon := testKeaDHCPDaemon{
ID: app.Daemons[0].KeaDaemon.KeaDHCPDaemon.ID,
KeaDaemonID: app.Daemons[0].KeaDaemon.KeaDHCPDaemon.KeaDaemonID,
Stats: testKeaDHCPDaemonStats{
RPS1: 1000,
RPS2: 2000,
},
}
_, err = db.Model(&keaDaemon).WherePK().Update()
require.NoError(t, err)

// Get the daemon. It uses float32 data types for RPS.
// Let's make sure it is fetched without errors and the
// values are correctly cast to float32.
daemons, err := GetDaemonsByIDs(db, []int64{daemon.ID})
require.NoError(t, err)
require.Len(t, daemons, 1)
daemon = &daemons[0]
require.NotNil(t, daemon.KeaDaemon)
require.NotNil(t, daemon.KeaDaemon.KeaDHCPDaemon)
require.NotNil(t, daemon.KeaDaemon.KeaDHCPDaemon.Stats)
require.Equal(t, float32(1000), daemon.KeaDaemon.KeaDHCPDaemon.Stats.RPS1)
require.Equal(t, float32(2000), daemon.KeaDaemon.KeaDHCPDaemon.Stats.RPS2)
}
8 changes: 2 additions & 6 deletions webui/src/app/dashboard/dashboard.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,8 @@ <h1 class="section-heading">Services Status</h1>
}"
></i>
</td>
<!--
Received RPS values must be divided by 100 because they are multipled
by 100 by the server to obtain two digit precision.
-->
<td pTooltip="{{ daemonRpsTooltip(d, 1) }}">{{ d.rps1 / 100 | number: '1.0-2' }}</td>
<td pTooltip="{{ daemonRpsTooltip(d, 2) }}">{{ d.rps2 / 100 | number: '1.0-2' }}</td>
<td pTooltip="{{ daemonRpsTooltip(d, 1) }}">{{ d.rps1 | number: '1.0-2' }}</td>
<td pTooltip="{{ daemonRpsTooltip(d, 2) }}">{{ d.rps2 | number: '1.0-2' }}</td>
<td>
<span *ngIf="d.haEnabled && d.haOverview && d.haOverview.length > 0">
<a routerLink="/apps/kea/{{ d.appId }}">
Expand Down
6 changes: 3 additions & 3 deletions webui/src/app/dashboard/dashboard.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ describe('DashboardComponent', () => {
monitored: true,
name: 'dhcp4',
uptime: 3652,
rps1: 152,
rps2: 34,
rps1: 1.5212,
rps2: 0.3458,
},
],
sharedNetworks4: {
Expand Down Expand Up @@ -456,6 +456,6 @@ describe('DashboardComponent', () => {
expect(rows.length).toBe(2)

expect(rows[1].nativeElement.innerText).toContain('1.52')
expect(rows[1].nativeElement.innerText).toContain('0.34')
expect(rows[1].nativeElement.innerText).toContain('0.35')
})
})

0 comments on commit c4a394d

Please sign in to comment.