Skip to content
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

fix: reader don't exit in some cases. #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zey1996
Copy link

@zey1996 zey1996 commented Mar 19, 2024

I notice cdc can't change to new generation in some case.
I think it's a bug.

https://github.com/zey1996/scylla-cdc-go/blob/e99bcf197b49a2603acc317875f5c6d62db9a0e2/stream_batch.go#L160

when we close reader, sbr.interruptCh be writed a struct. but ,if sbr.reachedEndOfTheGeneration(wnd.begin) return true.
the reader will never be close.

@zey1996 zey1996 changed the title fix: reader don't exit in some cases. fix: reader don't exit in some cases. Mar 19, 2024
@@ -299,7 +299,7 @@ func (sbr *streamBatchReader) reachedEndOfTheGeneration(windowEnd gocql.UUID) bo

func (sbr *streamBatchReader) close(processUntil gocql.UUID) {
sbr.endTimestamp.Store(processUntil)
sbr.interruptCh <- struct{}{}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, if you call sbr.close() with timestamp from future, it will spin in the following cycle without break consuming all cpu available:

for {
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(time.Until(delayUntil)):
break delay
case <-sbr.interruptCh:
if sbr.reachedEndOfTheGeneration(wnd.begin) {
break outer
}
}
}

@dkropachev
Copy link
Collaborator

I notice cdc can't change to new generation in some case. I think it's a bug.

Do you have any steps how to reproduce it, logs maybe ?

when we close reader, sbr.interruptCh be writed a struct. but ,if sbr.reachedEndOfTheGeneration(wnd.begin) return true. the reader will never be close.

Could you please clarify this statement, I don't see any pathway for it to happen.

@dkropachev dkropachev self-assigned this Aug 17, 2024
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

Successfully merging this pull request may close these issues.

2 participants