Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RawResponse is never cached. #339

Closed
joshua-rutherford opened this issue Aug 10, 2020 · 1 comment
Closed

RawResponse is never cached. #339

joshua-rutherford opened this issue Aug 10, 2020 · 1 comment

Comments

@joshua-rutherford
Copy link

The implementation of caching in #300 is not actually being used. There are the following issues:

  1. RawResponse.marshaledResponse is never set so nil should always be returned within this block: https://github.com/envoyproxy/go-control-plane/blob/master/pkg/cache/v2/cache.go#L108. So things should be breaking but aren't because:
  2. RawResponse.isResourceMarshaled will never be true with this check because the RawResponse.GetDiscoveryResponse method operates on the value of the raw response, not a pointer. Therefore, the RawResponse value in the method is not actually the same that the method is invoked upon (it is a copy).

The program below demonstrates issue number two above:

package main

import (
	"fmt"
)

type Structure struct {
	Boolean bool
}

func (s Structure) ToggleV() bool {
	s.Boolean = !s.Boolean
	return s.Boolean
}

func (s *Structure) ToggleP() bool {
	s.Boolean = !s.Boolean
	return s.Boolean
}

func main() {

	value := Structure{}

	for i := 0; i < 10; i++ {
		fmt.Printf("value: %t\n", value.ToggleV())
	}

	pointer := &Structure{}

	for i := 0; i < 10; i++ {
		fmt.Printf("pointer: %t\n", pointer.ToggleP())
	}
}

Which outputs:

value: true
value: true
value: true
value: true
value: true
value: true
value: true
value: true
value: true
value: true
pointer: true
pointer: false
pointer: true
pointer: false
pointer: true
pointer: false
pointer: true
pointer: false
pointer: true
pointer: false
@kyessenov
Copy link
Contributor

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants