Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants