Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
raj-prince committed Sep 13, 2024
1 parent b107155 commit e8a39d2
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 40 deletions.
46 changes: 23 additions & 23 deletions storage/util/dynamic_delay.go → storage/dynamic_delay.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package util
package storage

import (
"fmt"
Expand All @@ -21,11 +21,11 @@ import (
"time"
)

// Delay dynamically calculates the delay at a fixed percentile, based on
// dynamicDelay dynamically calculates the delay at a fixed percentile, based on
// delay samples.
//
// Delay is goroutine-safe.
type Delay struct {
// dynamicDelay is goroutine-safe.
type dynamicDelay struct {
increaseFactor float64
decreaseFactor float64
minDelay time.Duration
Expand All @@ -36,20 +36,20 @@ type Delay struct {
mu *sync.RWMutex
}

// NewDelay returns a Delay.
// NewDynamicDelay returns a dynamicDelay.
//
// targetPercentile is the desired percentile to be computed. For example, a
// targetPercentile of 0.99 computes the delay at the 99th percentile. Must be
// in the range [0, 1].
//
// increaseRate (must be > 0) determines how many Increase calls it takes for
// increaseRate (must be > 0) determines how many increase calls it takes for
// Value to double.
//
// initialDelay is the start value of the delay.
//
// Decrease can never lower the delay past minDelay, Increase can never raise
// decrease can never lower the delay past minDelay, increase can never raise
// the delay past maxDelay.
func NewDelay(targetPercentile float64, increaseRate float64, initialDelay, minDelay, maxDelay time.Duration) (*Delay, error) {
func newDynamicDelay(targetPercentile float64, increaseRate float64, initialDelay, minDelay, maxDelay time.Duration) (*dynamicDelay, error) {
if targetPercentile < 0 || targetPercentile > 1 {
return nil, fmt.Errorf("invalid targetPercentile (%v): must be within [0, 1]", targetPercentile)
}
Expand Down Expand Up @@ -77,7 +77,7 @@ func NewDelay(targetPercentile float64, increaseRate float64, initialDelay, minD
decreaseFactor = 0.9999
}

return &Delay{
return &dynamicDelay{
increaseFactor: increaseFactor,
decreaseFactor: decreaseFactor,
minDelay: minDelay,
Expand All @@ -87,7 +87,7 @@ func NewDelay(targetPercentile float64, increaseRate float64, initialDelay, minD
}, nil
}

func (d *Delay) increase() {
func (d *dynamicDelay) unsafeIncrease() {
v := time.Duration(float64(d.value) * d.increaseFactor)
if v > d.maxDelay {
d.value = d.maxDelay
Expand All @@ -96,15 +96,15 @@ func (d *Delay) increase() {
}
}

// Increase notes that the operation took longer than the delay returned by Value.
func (d *Delay) Increase() {
// increase notes that the operation took longer than the delay returned by Value.
func (d *dynamicDelay) increase() {
d.mu.Lock()
defer d.mu.Unlock()

d.increase()
d.unsafeIncrease()
}

func (d *Delay) decrease() {
func (d *dynamicDelay) unsafeDecrease() {
v := time.Duration(float64(d.value) * d.decreaseFactor)
if v < d.minDelay {
d.value = d.minDelay
Expand All @@ -113,37 +113,37 @@ func (d *Delay) decrease() {
}
}

// Decrease notes that the operation completed before the delay returned by Value.
func (d *Delay) Decrease() {
// decrease notes that the operation completed before the delay returned by getValue.
func (d *dynamicDelay) decrease() {
d.mu.Lock()
defer d.mu.Unlock()

d.decrease()
d.unsafeDecrease()
}

// Update notes that the RPC either took longer than the delay or completed
// before the delay, depending on the specified latency.
func (d *Delay) Update(latency time.Duration) {
func (d *dynamicDelay) Update(latency time.Duration) {
d.mu.Lock()
defer d.mu.Unlock()

if latency > d.value {
d.increase()
d.unsafeIncrease()
} else {
d.decrease()
d.unsafeDecrease()
}
}

// Value returns the desired delay to wait before retry the operation.
func (d *Delay) Value() time.Duration {
// getValue returns the desired delay to wait before retry the operation.
func (d *dynamicDelay) getValue() time.Duration {
d.mu.RLock()
defer d.mu.RUnlock()

return d.value
}

// PrintDelay prints the state of delay, helpful in debugging.
func (d *Delay) PrintDelay() {
func (d *dynamicDelay) printDelay() {
d.mu.RLock()
defer d.mu.RUnlock()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF

package util
package storage

import (
"math"
Expand All @@ -22,34 +22,34 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
)

func applySamples(numSamples int, expectedValue float64, rnd *rand.Rand, d *Delay) int {
func applySamples(numSamples int, expectedValue float64, rnd *rand.Rand, d *dynamicDelay) int {
var samplesOverThreshold int
for i := 0; i < numSamples; i++ {
randomDelay := time.Duration(-math.Log(rnd.Float64()) * expectedValue * float64(time.Second))
if randomDelay > d.Value() {
if randomDelay > d.getValue() {
samplesOverThreshold++
d.Increase()
d.increase()
} else {
d.Decrease()
d.decrease()
}
}
return samplesOverThreshold
}

func applySamplesWithUpdate(numSamples int, expectedValue float64, rnd *rand.Rand, d *Delay) {
func applySamplesWithUpdate(numSamples int, expectedValue float64, rnd *rand.Rand, d *dynamicDelay) {
for i := 0; i < numSamples; i++ {
randomDelay := time.Duration(-math.Log(rnd.Float64()) * expectedValue * float64(time.Second))
d.Update(randomDelay)
}
}

func TestNewDelay(t *testing.T) {
d, err := NewDelay(1-0.01, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
d, err := newDynamicDelay(1-0.01, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
if err != nil {
t.Fatal(err)
}

want := &Delay{
want := &dynamicDelay{
increaseFactor: 1.047294,
decreaseFactor: 0.999533,
minDelay: 1 * time.Millisecond,
Expand Down Expand Up @@ -84,7 +84,7 @@ func TestNewDelay(t *testing.T) {

func TestConvergence99(t *testing.T) {
// d should converge to the 99-percentile value.
d, err := NewDelay(1-0.01, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
d, err := newDynamicDelay(1-0.01, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestConvergence99(t *testing.T) {

func TestConvergence90(t *testing.T) {
// d should converge to the 90-percentile value.
d, err := NewDelay(1-0.1, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
d, err := newDynamicDelay(1-0.1, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
if err != nil {
t.Fatal(err)
}
Expand All @@ -145,31 +145,31 @@ func TestConvergence90(t *testing.T) {
}

func TestOverflow(t *testing.T) {
d, err := NewDelay(1-0.1, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
d, err := newDynamicDelay(1-0.1, 15, 1*time.Millisecond, 1*time.Millisecond, 1*time.Hour)
if err != nil {
t.Fatal(err)
}

n := 10000
for i := 0; i < n; i++ {
d.Increase()
d.increase()
}
t.Log(d.Value())
t.Log(d.getValue())
for i := 0; i < 100*n; i++ {
d.Decrease()
d.decrease()
}
if got, want := d.Value(), 1*time.Millisecond; got != want {
if got, want := d.getValue(), 1*time.Millisecond; got != want {
t.Fatalf("unexpected d.Value: got %v, want %v", got, want)
}
}

func TestInvalidArgument(t *testing.T) {
_, err := NewDelay(1-0.1, 15, 1*time.Millisecond, 2*time.Hour, 1*time.Hour)
_, err := newDynamicDelay(1-0.1, 15, 1*time.Millisecond, 2*time.Hour, 1*time.Hour)
if err == nil {
t.Fatal("unexpected, should throw error as minDelay is greater than maxDelay")
}

_, err = NewDelay(1-0.1, 0, 1*time.Millisecond, 2*time.Hour, 1*time.Hour)
_, err = newDynamicDelay(1-0.1, 0, 1*time.Millisecond, 2*time.Hour, 1*time.Hour)
if err == nil {
t.Fatal("unexpected, should throw error as increaseRate can't be zero")
}
Expand Down

0 comments on commit e8a39d2

Please sign in to comment.