When set disable_http_ac_validation, enable_ac_key_instance_mangling will not work

[hyphennn]

Hi, there. I'm using bazel-remote as our cache, my command to run progress is:

./bazel-bin/bazel-remote_/bazel-remote  \
  --dir /tmp/bazel-cache/ --max_size 200 --disable_http_ac_validation=true \
  --enable_endpoint_metrics=true --enable_ac_key_instance_mangling=true

And I find that the enable_ac_key_instance_mangling doesn't work: no matter how I change instance in .bazelrc, it always hit all caches.

I find in server/http.go Line 215:

	if h.mangleACKeys && kind == cache.AC {
		hash = cache.TransformActionCacheKey(hash, instance, h.accessLogger)
	}

So, when disable_http_ac_validation==true, the function TransformActionCacheKey will never work.

I forked this repo, and modified these code, it seems work well.

So, here is my question: Is this by design? If not, I am happy to offer my pr to fix it.

[mostynb]

Hi, I'm not sure I understand the problem yet, but these two configuration settings should be independent:

• --enable_ac_key_instance_mangling combines non-empty instance names with the original action cache key to form a new/different action cache key. ie so two requests with the same hash but different non-empty instance names will refer to two different action cache entries on disk. That happens in the block you mentioned:

  bazel-remote/server/http.go

  Line 215 in a563ac2

  if h.mangleACKeys && kind == cache.AC {

• --disable_http_ac_validation=true causes action cache entries to be returned immediately if they're found on disk. The default is to check that the entry is a valid ActionResult protobuf message and that all the CAS blobs that it references also exist in the cache, and return 404 if any of those conditions are false. This happens after the mangling (if that is enabled):

  bazel-remote/server/http.go

  Line 227 in a563ac2

  if h.validateAC && kind == cache.AC {

Could you provide a test case that shows the problem?

[hyphennn]

Sure. I'll give an example, and I think it will be easily for you to reproduct.

Here is my command to start a bazel-remote server locally:

./bazel-bin/bazel-remote_/bazel-remote  \
  --dir /tmp/bazel-cache/ --max_size 200 --disable_http_ac_validation=true \
  --enable_ac_key_instance_mangling=true

Than I run bazel build with this remote cache and instance named 'test'

bazel build //:bazel-remote --remote_cache=http://localhost:8080/test
bazel clean
bazel build //:bazel-remote --remote_cache=http://localhost:8080/test

The second 'bazel build' is aim to validate cache hit. Of course all caches hit.In my machine, it is 264/270 as below:

INFO: 270 processes: 264 remote cache hit, 6 internal.
INFO: Build completed successfully, 270 total actions

Than I changed the instance to 'test2' with below command:

bazel clean
bazel build //:bazel-remote --remote_cache=http://localhost:8080/test2

As expected, it should not hit any cache. However, it still hits all cache:

INFO: 270 processes: 264 remote cache hit, 6 internal.
INFO: Build completed successfully, 270 total actions

I think it is easy to find the root cause. In below code, only when kind==AC, the hash will be transformed.

bazel-remote/server/http.go

Lines 215 to 217 in a563ac2

	if h.mangleACKeys && kind == cache.AC {
	hash = cache.TransformActionCacheKey(hash, instance, h.accessLogger)
	}

The kind and hash are decided in below code:

bazel-remote/server/http.go

Line 208 in a563ac2

kind, hash, instance, err := parseRequestURL(r.URL.Path, h.validateAC)

Then let's see the func parseRequestURL:

bazel-remote/server/http.go

Lines 114 to 118 in a563ac2

	if validateAC {
	return cache.AC, hash, instance, nil
	}
	return cache.RAW, hash, instance, nil

Only when validateAC==true, it will return AC, otherwise it will retuen RAW. And the variable validateAC is decided by settings --disable_http_ac_validation.

So, when disable_http_ac_validation==true, the kind will permanently be RAW, and the hash will never be transformed.

[hyphennn]

I think it's not hard to fix this problem. And I provide my pr, which has run in our bazel-remote cache with my forked repo, in #792 .

[mostynb]

Merged- thanks.