Skip to content

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)
}

@hezhangjian
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.

@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())
}

@hezhangjian hezhangjian force-pushed the main branch 2 times, most recently from b27dcfa to cc17f8e Compare August 4, 2024 10:16
@hezhangjian hezhangjian 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]>
@hezhangjian hezhangjian merged commit 4374b8b into openGemini:main Aug 4, 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.

for select usage error

3 participants