tvalentyn commented on code in PR #38246:
URL: https://github.com/apache/beam/pull/38246#discussion_r3113654749
##########
sdks/python/container/boot.go:
##########
@@ -314,23 +314,47 @@ func launchSDKProcess() error {
var wg sync.WaitGroup
wg.Add(len(workerIds))
- for _, workerId := range workerIds {
- go func(workerId string) {
- defer wg.Done()
-
- bufLogger := tools.NewBufferedLogger(logger)
- errorCount := 0
- for {
- childPids.mu.Lock()
- if childPids.canceled {
- childPids.mu.Unlock()
- return
- }
- logger.Printf(ctx, "Executing Python (worker
%v): python %v", workerId, strings.Join(args, " "))
- cmd :=
StartCommandEnv(map[string]string{"WORKER_ID": workerId}, os.Stdin, bufLogger,
bufLogger, "python", args...)
- childPids.v = append(childPids.v,
cmd.Process.Pid)
- childPids.mu.Unlock()
-
+for _, workerId := range workerIds {
+ go func(workerId string) {
+ defer wg.Done()
+
+ workerCtx := grpcx.WriteWorkerID(ctx, workerId)
Review Comment:
1. Does this WriteWorkerID call rewrite the metadata or it appends it every
time?
2. This seems to conflict with the ID used in Line 179, I wonder if this
would cause unintended side-effects for logs emitted within boot.go
3. For my understanding , does this line cause a certain side effect for the
logger we create in line 327? Or this only impacts the logging on line 341
(which seemingly only prints 1 line, announcing which Python process was
launched.
--
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]
