TeamMaster#findServers creates incorrect copy of the ComputeRequests

[radike]

The problem is at line 73 in file master/resource/service_discovery/master/master_allocation.go. It stores pointer of the loop variable request, instead of storing pointer to the i-th element of the req.Requests.

[chrislusf]

Thanks a lot for debugging this! You can send a pull request next time, which will attribute the credit to you!

[radike]

Hi @chrislusf,

I think that the committed fix doesn't solve the issue. The local variable req is a pointer to the loop variable, so the slice still contains only pointers to the loop variable. See http://play.golang.org/p/_fos0OJUkq

I have created additional allocation test that verifies the allocation. There is one server with 2048 MB of memory, and 3 requests that differs in the requested memory: 1024 MB, 512 MB, 256 MB.

The expected is: master_allocation_test.go:236: Result: &{Allocations:[{Location:{DataCenter:dc1 Rack:rack1 Server:server1 Port:1111} Allocated:CPUCount 1 Level 1 Memory 1024 MB} {Location:{DataCenter:dc1 Rack:rack1 Server:server1 Port:1111} Allocated:CPUCount 1 Level 1 Memory 512 MB} {Location:{DataCenter:dc1 Rack:rack1 Server:server1 Port:1111} Allocated:CPUCount 1 Level 1 Memory 256 MB}] Error:}

but the output before the proposed fix is: master_allocation_test.go:236: Result: &{Allocations:[{Location:{DataCenter:dc1 Rack:rack1 Server:server1 Port:1111} Allocated:CPUCount 1 Level 1 Memory 256 MB} {Location:{DataCenter:dc1 Rack:rack1 Server:server1 Port:1111} Allocated:CPUCount 1 Level 1 Memory 256 MB} {Location:{DataCenter:dc1 Rack:rack1 Server:server1 Port:1111} Allocated:CPUCount 1 Level 1 Memory 256 MB}] Error:}

[chrislusf]

That's right. Please create a PR to change it from:

for _, request := range []int{1, 2, 3} {
    req := &request
    requests = append(requests, req)
}

to:

for _, request := range []int{1, 2, 3} {
    req := request
    requests = append(requests, &req)
}