use gocloud to support distributed lambda_store #322
Conversation
| @@ -42,38 +36,52 @@ type LambdaEntry struct { | |||
| Lock *sync.Mutex | |||
| } | |||
|
|
|||
| func NewLambdaStore(storePath string, pool *cloudvm.WorkerPool) (*LambdaStore, error) { | |||
| trashDir := filepath.Join(storePath, ".trash") | |||
| func NewLambdaStore(storeURL string, pool *cloudvm.WorkerPool) (*LambdaStore, error) { | |||
There was a problem hiding this comment.
Can we document with the comment why we may or may not have a pool, and the behavior in each case?
| return fmt.Errorf("failed to upload to blob storage: %w", err) | ||
| } | ||
|
|
||
| if err := writer.Close(); err != nil { |
There was a problem hiding this comment.
We should use the defer pattern so it gets closed even if io.Copy fails.
| @@ -264,8 +296,8 @@ func (s *LambdaStore) removeFromRegistry(funcName string) error { | |||
|
|
|||
| // Background deletion | |||
| go func() { | |||
| if err := os.Remove(trashPath); err != nil { | |||
| log.Printf("warning: failed to remove %s from trash: %v", trashPath, err) | |||
| if err := s.bucket.Delete(context.Background(), funcName+".tar.gz"); err != nil { | |||
There was a problem hiding this comment.
It seems like we hardcode .tar.gz in a lot of places. Might be a pain later if we use a different format. At the least, how about a const for now?
|
|
||
| // If not cached, try to load from blob storage | ||
| if err := s.loadConfigAndRegister(funcName); err != nil { | ||
| return nil, fmt.Errorf("lambda %q not found", funcName) |
There was a problem hiding this comment.
How do we know the err is not found, and not something else? Probably better to propagate with %w.
| @@ -175,7 +175,7 @@ func GetDefaultWorkerConfig(olPath string) (*Config, error) { | |||
| Server_mode: "lambda", | |||
| Worker_url: "localhost", | |||
| Worker_port: "5000", | |||
| Registry: registryDir, | |||
| Registry: "file://" + registryDir, | |||
There was a problem hiding this comment.
Should we have a comment about why the protocol prefix is necessary?
| @@ -278,10 +278,11 @@ func Main() (err error) { | |||
| http.HandleFunc(PPROF_CPU_STOP_PATH, PprofCpuStop) | |||
|
|
|||
| // Initialize LambdaStore for registry | |||
| log.Printf("Worker: Initializing LambdaStore with Registry = %s", common.Conf.Registry) | |||
There was a problem hiding this comment.
Let's have quotes around the %s, so it is obvious the problem if empty.
| lambdaStore, err = lambdastore.NewLambdaStore(common.Conf.Registry, nil) | ||
| if err != nil { | ||
| os.Remove(pidPath) | ||
| return fmt.Errorf("failed to initialize lambda store: %v", err) | ||
| return fmt.Errorf("failed to initialize lambda store at %s: %v", common.Conf.Registry, err) |
| } | ||
| attrs, err := cp.bucket.Attributes(context.Background(), key) | ||
| if err == nil { | ||
| version := attrs.ModTime.String() |
There was a problem hiding this comment.
Why convert the ModTime to a string, instead of just storing the version as a timestamp? Do you envision it being represented differently at some point?
No description provided.