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.

Sign up for GitHub

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

Jump to bottom

proposal: bufio: add Writer.UnwrittenData #59106

Closed
wuqinqiang opened this issue Mar 18, 2023 · 7 comments
Closed

proposal: bufio: add Writer.UnwrittenData #59106

wuqinqiang opened this issue Mar 18, 2023 · 7 comments
Labels
Proposal
Milestone
Proposal

Comments

@wuqinqiang
Copy link

wuqinqiang commented Mar 18, 2023 •
edited

Overview:
Currently, bufio.Writer does not provide a way to retrieve data that has not been successfully written. If a Flush() call fails, data in the buffer cannot be written to the underlying io.Writer.

Even if flush is called continuously, it will always fail.

func (b *Writer) Flush() error {
	if b.err != nil {
		return b.err
	}
  //....
}

We propose adding an UnwrittenData() method to allow users to retrieve data that has not been successfully written in case of a failed Flush(), enabling additional processing or retrying.

Motivation:
In some cases, Flush() may fail, for example, due to a network interruption or the underlying io.Writer closing. This could result in some data in the buffer not being written to the underlying io.Writer and being lost, which could cause inconsistencies in the application state or data loss. Therefore, users need a way to retrieve data in the buffer that has not been successfully written.

Proposal:
We propose adding an UnwrittenData() method that returns data in the buffer that has not been successfully written.

type Writer struct {
// ...
}

func (w *Writer) UnwrittenData() []byte {
return w.buf[:w.n]
}

If the data in the buffer has been successfully written to the underlying io.Writer before Flush() is called, UnwrittenData() returns an empty byte slice.

Compatibility:
This proposal has no impact on compatibility with existing code, as the new method does not modify existing interfaces or behavior.

Implementation:
The proposed method only needs to return data in the buffer that has not been successfully written, and does not need to modify any existing code or behavior.

Example:
Here is an example of how the UnwrittenData() method could be used:

	w := bufio.NewWriter(conn)
	// Write some data to the buffer
	w.WriteString("hello")
	w.WriteString("world")

	// Try to write to the underlying io.Writer, retrieve any unwritten data on failure and handle it
	if err := w.Flush(); err != nil {
		unwrittenData := w.UnwrittenData()
		log.Printf("Unwritten data: %v\n", unwrittenData)
		// Try to rewrite unwritten data by conn
		if _, err := conn.Write(unwrittenData); err != nil {
			log.Printf("Write unwritten data failed: %v\n", err)
		}
	}
@gopherbot gopherbot added this to the Proposal milestone Mar 18, 2023
@ianlancetaylor
Copy link
Contributor

CC @dsnet

@dsnet
Copy link
Member

dsnet commented Mar 19, 2023

The idea itself generally sounds fine, but it's not clear to me what this is trying to solve:

  1. Is the goal just to log the unwritten data? That's not typically done as logging arbitrary data could leak sensitive information and also make logs very verbose.
  2. Is the goal to have another shot at trying to write the data (by calling conn.Write(unwrittenData)? Then maybe instead we should have a mode where you can disable sticky error values, so you can just call Flush again however many times you want.

If we do add this, I suggest we name it UnflushedBuffer as "unflushed" matches the naming of the "Flush" method and "buffer" matches the existing "AvailableBuffer" method.

@wuqinqiang
Copy link
Author

wuqinqiang commented Mar 20, 2023 •
edited

My global is to address the issue of allowing users can flush data again when a flush error occurs, without being affected by previous errors.

Given that, I fully agree with adding a new mod.

@rsc
Copy link
Contributor

rsc commented May 10, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc changed the title proposal: bufio: Add UnwrittenData() method to bufio.Writer proposal: bufio: add Writer.UnwrittenData May 10, 2023
@rsc
Copy link
Contributor

rsc commented May 10, 2023

This does not sound like a great idea. Taking one of the motivating use cases, if a network connection is interrupted, there is no guarantee that earlier Write calls that succeeded were actually received on the other side. Being able to find the data that isn't yet flushed does not recover all the data that has been "lost". The same is true for writes to a file in a file system. If the kernel buffers some writes and then the disk has an I/O error, perhaps a future write will fail, but the earlier writes have failed too.

If you run a sequence like Write+Flush+Close+Sync and they all succeed, or if you do Write+Flush+read an ack from the other side, then you know everything worked. If any of those fail, it is quite rare to be able to reliably tell what has and has not been received/written/etc.

Adding UnwrittenData seems like it encourages people to think the available guarantees are stronger than they really are.

@rsc
Copy link
Contributor

rsc commented May 17, 2023

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented May 24, 2023

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal
Projects
Proposals
Status: Declined
Milestone
Proposal
Development

No branches or pull requests
5 participants
@rsc @ianlancetaylor @dsnet @gopherbot @wuqinqiang