lostluck commented on a change in pull request #15483: URL: https://github.com/apache/beam/pull/15483#discussion_r706494092
########## File path: sdks/go/pkg/beam/core/runtime/harness/statecache/statecache.go ########## @@ -0,0 +1,215 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package statecache implements the state caching feature described by the +// Beam Fn API +// +// The Beam State API and the intended caching behavior are described here: +// https://docs.google.com/document/d/1BOozW0bzBuz4oHJEuZNDOHdzaV5Y56ix58Ozrqm2jFg/edit#heading=h.7ghoih5aig5m +package statecache + +import ( + "sync" + + "github.com/apache/beam/sdks/v2/go/pkg/beam/core/runtime/exec" + "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors" + fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1" +) + +type token string + +// SideInputCache stores a cache of reusable inputs for the purposes of +// eliminating redundant calls to the runner during execution of ParDos +// using side inputs. +// +// A SideInputCache should be initialized when the SDK harness is initialized, +// creating storage for side input caching. On each ProcessBundleRequest, +// the cache will process the list of tokens for cacheable side inputs and +// be queried when side inputs are requested in bundle execution. Once a +// new bundle request comes in the valid tokens will be updated and the cache +// will be re-used. In the event that the cache reaches capacity, a random, +// currently invalid cached object will be evicted. +type SideInputCache struct { + capacity int + mu sync.Mutex + cache map[token]exec.ReusableInput + idsToTokens map[string]token + validTokens map[token]int8 // Maps tokens to active bundle counts + metrics CacheMetrics +} + +type CacheMetrics struct { + Hits int64 + Misses int64 + Evictions int64 + InUseEvictions int64 +} + +// Init makes the cache map and the map of IDs to cache tokens for the +// SideInputCache. Should only be called once. Returns an error for +// non-positive capacities. +func (c *SideInputCache) Init(cap int) error { + if cap <= 0 { + return errors.Errorf("capacity must be a positive integer, got %v", cap) + } + c.mu.Lock() + defer c.mu.Unlock() + c.cache = make(map[token]exec.ReusableInput, cap) + c.idsToTokens = make(map[string]token) + c.validTokens = make(map[token]int8) + c.capacity = cap + return nil +} + +// SetValidTokens clears the list of valid tokens then sets new ones, also updating the mapping of +// transform and side input IDs to cache tokens in the process. Should be called at the start of every +// new ProcessBundleRequest. If the runner does not support caching, the passed cache token values +// should be empty and all get/set requests will silently be no-ops. +func (c *SideInputCache) SetValidTokens(cacheTokens ...fnpb.ProcessBundleRequest_CacheToken) { + c.mu.Lock() + defer c.mu.Unlock() + for _, tok := range cacheTokens { + // User State caching is currently not supported, so these tokens are ignored + if tok.GetUserState() != nil { + continue + } + s := tok.GetSideInput() + transformID := s.GetTransformId() + sideInputID := s.GetSideInputId() + t := token(tok.GetToken()) + c.setValidToken(transformID, sideInputID, t) + } +} + +// setValidToken adds a new valid token for a request into the SideInputCache struct +// by mapping the transform ID and side input ID pairing to the cache token. +func (c *SideInputCache) setValidToken(transformID, sideInputID string, tok token) { + idKey := transformID + sideInputID + c.idsToTokens[idKey] = tok + count, ok := c.validTokens[tok] + if !ok { + c.validTokens[tok] = 1 + } else { + c.validTokens[tok] = count + 1 + } Review comment: I like the structural parity between this method and `decrementTokenCount`, but do want to call something about about Go maps. FYI Go Maps automatically return the zero value for the value type. See https://tour.golang.org/moretypes/22 In this case, it's 0. In other words, this block can be simplified a little. (but as I said, structural parity). ########## File path: sdks/go/pkg/beam/core/runtime/harness/statecache/statecache.go ########## @@ -0,0 +1,215 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package statecache implements the state caching feature described by the +// Beam Fn API +// +// The Beam State API and the intended caching behavior are described here: +// https://docs.google.com/document/d/1BOozW0bzBuz4oHJEuZNDOHdzaV5Y56ix58Ozrqm2jFg/edit#heading=h.7ghoih5aig5m +package statecache + +import ( + "sync" + + "github.com/apache/beam/sdks/v2/go/pkg/beam/core/runtime/exec" + "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors" + fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1" +) + +type token string + +// SideInputCache stores a cache of reusable inputs for the purposes of +// eliminating redundant calls to the runner during execution of ParDos +// using side inputs. +// +// A SideInputCache should be initialized when the SDK harness is initialized, +// creating storage for side input caching. On each ProcessBundleRequest, +// the cache will process the list of tokens for cacheable side inputs and +// be queried when side inputs are requested in bundle execution. Once a +// new bundle request comes in the valid tokens will be updated and the cache +// will be re-used. In the event that the cache reaches capacity, a random, +// currently invalid cached object will be evicted. +type SideInputCache struct { + capacity int + mu sync.Mutex + cache map[token]exec.ReusableInput + idsToTokens map[string]token + validTokens map[token]int8 // Maps tokens to active bundle counts + metrics CacheMetrics +} + +type CacheMetrics struct { + Hits int64 + Misses int64 + Evictions int64 + InUseEvictions int64 +} + +// Init makes the cache map and the map of IDs to cache tokens for the +// SideInputCache. Should only be called once. Returns an error for +// non-positive capacities. +func (c *SideInputCache) Init(cap int) error { + if cap <= 0 { + return errors.Errorf("capacity must be a positive integer, got %v", cap) + } + c.mu.Lock() + defer c.mu.Unlock() + c.cache = make(map[token]exec.ReusableInput, cap) + c.idsToTokens = make(map[string]token) + c.validTokens = make(map[token]int8) + c.capacity = cap + return nil +} + +// SetValidTokens clears the list of valid tokens then sets new ones, also updating the mapping of +// transform and side input IDs to cache tokens in the process. Should be called at the start of every +// new ProcessBundleRequest. If the runner does not support caching, the passed cache token values +// should be empty and all get/set requests will silently be no-ops. +func (c *SideInputCache) SetValidTokens(cacheTokens ...fnpb.ProcessBundleRequest_CacheToken) { + c.mu.Lock() + defer c.mu.Unlock() + for _, tok := range cacheTokens { + // User State caching is currently not supported, so these tokens are ignored + if tok.GetUserState() != nil { + continue + } + s := tok.GetSideInput() + transformID := s.GetTransformId() + sideInputID := s.GetSideInputId() + t := token(tok.GetToken()) + c.setValidToken(transformID, sideInputID, t) + } +} + +// setValidToken adds a new valid token for a request into the SideInputCache struct +// by mapping the transform ID and side input ID pairing to the cache token. +func (c *SideInputCache) setValidToken(transformID, sideInputID string, tok token) { + idKey := transformID + sideInputID + c.idsToTokens[idKey] = tok + count, ok := c.validTokens[tok] + if !ok { + c.validTokens[tok] = 1 + } else { + c.validTokens[tok] = count + 1 + } +} + +// CompleteBundle takes the cache tokens passed to set the valid tokens and decrements their +// usage count for the purposes of maintaining a valid count of whether or not a value is +// still in use. Should be called once ProcessBundle has completed. +func (c *SideInputCache) CompleteBundle(cacheTokens ...fnpb.ProcessBundleRequest_CacheToken) { + c.mu.Lock() + defer c.mu.Unlock() + for _, tok := range cacheTokens { + // User State caching is currently not supported, so these tokens are ignored + if tok.GetUserState() != nil { + continue + } + t := token(tok.GetToken()) + c.decrementTokenCount(t) + } +} + +// decrementTokenCount decrements the validTokens entry for +// a given token by 1. Should only be called when completing +// a bundle. +func (c *SideInputCache) decrementTokenCount(tok token) { + count := c.validTokens[tok] + if count == 1 { + delete(c.validTokens, tok) + } else { + c.validTokens[tok] = count - 1 + } +} + +func (c *SideInputCache) makeAndValidateToken(transformID, sideInputID string) (token, bool) { + idKey := transformID + sideInputID + // Check if it's a known token + tok, ok := c.idsToTokens[idKey] + if !ok { + return "", false + } + return tok, c.isValid(tok) +} + +// QueryCache takes a transform ID and side input ID and checking if a corresponding side +// input has been cached. A query having a bad token (e.g. one that doesn't make a known +// token or one that makes a known but currently invalid token) is treated the same as a +// cache miss. +func (c *SideInputCache) QueryCache(transformID, sideInputID string) exec.ReusableInput { + c.mu.Lock() + defer c.mu.Unlock() Review comment: I have a small concern around the QueryCache call in that at worst it means a lock call (and a serialization of cross bundle calls) for every element, but I expect the single value/in bundle cache + the benefits of this cache in general override that concern. However, that's something we can probably figure out a way to measure, or have an "off switch" for just in case, in a later PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
