-
Notifications
You must be signed in to change notification settings - Fork 2
feat: modify pool tests #153
Changes from 3 commits
17bb795
00a4344
0cd98cf
b6b03f9
5537699
778cd45
35646a8
fa59f39
a23963d
d2c669e
ce46ce5
ca5522d
27b62be
296eaec
eb1e8b8
2713f51
7375178
93135a7
bda8d0d
c8be27d
d52ef6e
61c82da
550cf5b
c0ea85c
608a668
ea1d62b
05c2b37
c1ab0e9
1975f49
78f3490
5e02c7f
d8ae01e
b647fab
0cf6c94
552ea1b
9eb9c18
af17595
310c079
8804f45
3f63a01
da9ad17
46b5374
ad399fb
1015a7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,90 @@ | ||
package caboose_test | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"math/rand" | ||
"net/url" | ||
"testing" | ||
"time" | ||
|
||
"github.com/filecoin-saturn/caboose" | ||
"github.com/filecoin-saturn/caboose/internal/util" | ||
"github.com/ipfs/go-cid" | ||
"github.com/multiformats/go-multicodec" | ||
) | ||
|
||
|
||
const ( | ||
nodesSize = 10 | ||
nodesPoolSize = caboose.PoolConsiderationCount | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to do this ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is used to change the pool size desired for testing. I reduced the pool size for testing cache affinity to make it easier to debug and get the tests passing. Once we get that passing we can set it to a high / realistic number again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, yeah that makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has already been structured so that the lower pool target is only used in the test context. A larger pool size target is used when running normally |
||
|
||
|
||
func TestPoolDynamics(t *testing.T) { | ||
ch := util.BuildCabooseHarness(t, 3, 3) | ||
|
||
ch := util.BuildCabooseHarness(t, nodesSize , 3) | ||
ch.StartOrchestrator() | ||
baseStatSize := 100 | ||
baseStatLatency := 100 | ||
ctx := context.Background() | ||
|
||
testCid, _ := cid.V1Builder{Codec: uint64(multicodec.Raw), MhType: uint64(multicodec.Sha2_256)}.Sum(testBlock) | ||
|
||
ch.FetchAndAssertSuccess(t, ctx, testCid) | ||
|
||
rand.New(rand.NewSource(0)) | ||
eps := ch.Endpoints | ||
controlGroup := make(map[string]string) | ||
|
||
rand.Shuffle(len(eps), func(i, j int) { | ||
eps[i], eps[j] = eps[j], eps[i] | ||
}) | ||
|
||
for _,ep := range(eps[:nodesPoolSize]) { | ||
url, _ := url.Parse(ep.Server.URL) | ||
controlGroup[url.Host] = ep.Server.URL | ||
|
||
} | ||
|
||
for i := 0; i < 1; i++ { | ||
nodes := ch.CabooseAllNodes | ||
goodNodes := make([]*caboose.Node, 0) | ||
badNodes := make([]*caboose.Node, 0) | ||
goodStats := util.NodeStats{ | ||
Start: time.Now().Add(-time.Second*6), | ||
Latency: float64(baseStatLatency) / float64(10), | ||
Size: float64(baseStatSize) * float64(10), | ||
} | ||
badStats := util.NodeStats{ | ||
Start: time.Now().Add(-time.Second*6), | ||
Latency: float64(baseStatLatency) * float64(10), | ||
Size: float64(baseStatSize) / float64(10), | ||
} | ||
for _,n := range(nodes.Nodes) { | ||
_, ok := controlGroup[n.URL] | ||
if ok { | ||
fmt.Println("Good", n.URL) | ||
|
||
goodNodes = append(goodNodes, n) | ||
} else { | ||
fmt.Println("Bad", n.URL) | ||
|
||
badNodes = append(badNodes, n) | ||
} | ||
} | ||
|
||
ch.RecordSuccesses(t, goodNodes, goodStats, 10) | ||
ch.RecordSuccesses(t, badNodes, badStats, 10) | ||
|
||
} | ||
|
||
|
||
ch.CaboosePool.DoRefresh() | ||
fmt.Println("Pool", ch.CabooseActiveNodes.Nodes) | ||
|
||
for _,n := range(ch.CabooseAllNodes.Nodes) { | ||
fmt.Println("Node", n.URL, "size", n.PredictedThroughput) | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willscott noticed a slight issue with the use of heaps here.
The heap will keep itself sorted upon insertion and removal of new nodes. However, the existing nodes in the heap have dynamically changing values and the heap does not continuously sort itself every time one of those values changes.
The strategy I've implemented here is to select the top N nodes from the heap, to use
.Pop()
n times then to retrieve the top nodes, then add them back using.Push()
. This is a similar flow to the.Fix()
method that exists on the heap which is meant to reposition an item in the heap if it changes its value. Not the most efficient, but we are dealing with fairly small heap sizes here.Also wondering if we still think that a priority queue is the best data structure to store the
AllNodes
tier? My understanding of the code / implementation is that theActiveNodes
is always a subset ofAllNodes
and they are not mutually exclusive sets like the previous "Main" and "Uknown" tier design.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willscott One issue I noticed with my changes is that since the nodes are temporarily removed from the heap, if there are any stats being written to them concurrently, that will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by 'fail'? a temporary dropped metric isn't the end of the world, potentially.
If not a priority queue for all nodes, how would you imagine keeping track of the whole set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What i mean by "fail" is that any concurrent operation that accesses the heap while the topN selection is occurring, it might get an incorrect list. I do agree with you though, it doesn't seem that damaging specially that the "AllNodes" set is just used for testing nodes.
Wondering if a regular array structure would work better here. It's simpler and gives us the functionality we need (top n nodes, random nodes, node removal). We're not really using the properties of a priority queue because the values of each node in the queue are changing constantly and we have to basically re-sort the nodes every time we need to make a selection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one note on golang semantics here: you have the
nh.lk.RLock()
above - that indicates you're taking a "read only lock" over the data structure - but the pop and push you're proposing in your change will do a read-write operation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tip here Will, always good to learn :)