Skip to content

message's Context doesn't save when a worker fetch the message in v3 #196

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

Open
FitzBogard opened this issue Aug 9, 2023 · 1 comment
Open

Comments

@FitzBogard
Copy link

FitzBogard commented Aug 9, 2023

func (c *Consumer) worker(ctx context.Context, workerID int32) {
	var lock *redislock.Lock
	defer func() {
		if lock != nil {
			_ = lock.Release(ctx)
		}
	}()

	timer := time.NewTimer(time.Minute)
	timer.Stop()

	for {
		if workerID >= atomic.LoadInt32(&c.numWorker) {
			return
		}
		if c.opt.WorkerLimit > 0 {
			lock = c.lockWorker(ctx, lock, workerID)
		}

		msg := c.waitMessage(ctx, timer)
		if msg == nil {
			if atomic.LoadInt32(&c.state) >= stateStoppingWorkers {
				return
			}
			continue
		}

                msg.Ctx = ctx // this place
		_ = c.Process(msg)
	}
}

if we use consumer's context, we will not use the redis queue's message's context, it's a specified context which definited by the sdk users, it may contains some their own context's value, so I think it's better to remove the line "msg.Ctx = ctx", let users use their context.

@philippgille
Copy link

I don't think removing that line is enough, because the msg.Ctx is not serialized, so not propagated through Redis, so it would be empty in the worker:

Ctx context.Context `msgpack:"-"`

But haven't tested.

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