influxdata / influxdata/influxdb-client-go
WriteAPIImpl.Flush is unnecessarily complex
- Dominant language
- Go
- Stars
- 653
- Forks
- 120
- Avg merge
- 3m
- Merged PRs (30d)
- 3
Description
`WriteAPIImpl.Flush()` appears to have an irrelevant bug, and to be overly complex for what it actually does, which does not include
flushing as implied by the documentation.
[waitForFlushing()](https://github.com/influxdata/influxdb-client-go/blob/4218b5069e003d9575d6d8221a43f7eed6e5f6ec/api/write.go#L114-L133) has two steps:
1. Loop until the goroutine animating `bufferProc()` has acknowledged that `bufferInfoCh` has length zero. **[This](https://github.com/influxdata/influxdb-client-go/blob/4218b5069e003d9575d6d8221a43f7eed6e5f6ec/api/write.go#L155) is probably an error, and the length of `bufferCh` is intended to be checked.**
2. Loop until the goroutine animating `writeProc()` has acknowledged that `writeCh` has length zero.
A fundamental problem is that `bufferCh`, `bufferInfoCh`, and `writeCh` are all unbuffered channels: their length will never be anything but zero. So the only effect of `waitForFlushing()` is transient synchronizations with the `bufferProc()` and `writeProc()` goroutines. https://github.com/pabigot/influxdb-client-go/commit/b9b12c8207e68f569bf81d5a7e3fba82ecc14ebe provides a simplified equivalent.
Since cleaning this up doesn't address the core problem of #289 I haven't submitted a PR, but I can do so if you wish.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in api/write.go with WriteAPIImpl.Flush(), waitForFlushing(), bufferProc(), and writeProc(). Compare the current synchronization with the simplified equivalent in commit b9b12c8207e68f569bf81d5a7e3fba82ecc14ebe, checking the noted bufferCh versus bufferInfoCh discrepancy. Done means the unnecessary complexity is removed while preserving the documented behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100