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: batch channel not close when client close #123

Merged
merged 1 commit into from
Aug 4, 2024

Conversation

cyruslo
Copy link
Contributor

@cyruslo cyruslo commented Jul 28, 2024

Fix #100

@cyruslo cyruslo force-pushed the main branch 2 times, most recently from a2222d4 to 4a98395 Compare July 28, 2024 15:21
@xuthus5
Copy link
Member

xuthus5 commented Jul 29, 2024

the client will block because the channel is not closed, a simple demo to verify the idea, End of data will never have a chance to print.

package main

import (
	"fmt"
	"time"
)

func reader(ch <-chan int) {
	for record := range ch {
		fmt.Println(record)
	}
	fmt.Println("End of data")
}

func main() {
	var ch = make(chan int)
	go reader(ch)
	for i := 0; i < 10; i++ {
		ch <- i
	}
	time.Sleep(time.Second * 10)
}

@shoothzj
Copy link
Member

Currently, we don't have any restrictions, so we can still call WritePointWithRp after closing it. It may contains some leak.

@xuthus5
Copy link
Member

xuthus5 commented Jul 29, 2024

Currently, we don't have any restrictions, so we can still call after closing it. It may contains some leak.WritePointWithRp

Close() semantics have no effect.

opengemini/write.go Outdated Show resolved Hide resolved
opengemini/write.go Outdated Show resolved Hide resolved
opengemini/write.go Outdated Show resolved Hide resolved
@xuthus5
Copy link
Member

xuthus5 commented Aug 4, 2024

Close method:

func (c *client) Close() error {
	c.batchContextCancel()
	c.dataChanMap.Range(func(key, value interface{}) bool {
		close(value.(chan *sendBatchWithCB))
		c.dataChanMap.Delete(key)
		return true
	})
	return nil
}

WritePointWithRp method on new line 58, add a context cancel judgment to prevent the program from writing data to a closed channel:

if c.batchContext.Err() != nil {
    return errors.New("batch write failed, error: " + c.batchContext.Err().Error())
}

@shoothzj shoothzj force-pushed the main branch 2 times, most recently from b27dcfa to cc17f8e Compare August 4, 2024 10:16
@shoothzj shoothzj changed the title fixed: for select usage error(#100) fix: batch channel not close when client close Aug 4, 2024
Signed-off-by: cyruslo <[email protected]>
Signed-off-by: ZhangJian He <[email protected]>
Co-authored-by: ZhangJian He <[email protected]>
Signed-off-by: ZhangJian He <[email protected]>
@shoothzj shoothzj merged commit 4374b8b into openGemini:main Aug 4, 2024
8 checks passed
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.

for select usage error
3 participants