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

go 1.19 causes different checksums in TestMultistoreSnapshot_Checksum #13373

Closed
tac0turtle opened this issue Sep 23, 2022 · 5 comments · Fixed by #13400
Closed

go 1.19 causes different checksums in TestMultistoreSnapshot_Checksum #13373

tac0turtle opened this issue Sep 23, 2022 · 5 comments · Fixed by #13400

Comments

@tac0turtle
Copy link
Member

Summary of Bug

with go 1.19 we witnessed a different hash being produced in TestMultistoreSnapshot_Checksum. We should identify and fix this if possible otherwise report it to the go team.

@odeke-em do you have any ideas?

@alexanderbez
Copy link
Contributor

alexanderbez commented Sep 23, 2022

For ref, here are the release notes for GO 1.19

https://tip.golang.org/doc/go1.19

We should look at Minor changes to the library.

@mmsqe
Copy link
Contributor

mmsqe commented Sep 23, 2022

seems extra bytes 010000ffff get removed by zlib in 1.19, diff comes from deflate close (1.18 vs 1.19)

@elias-orijtech
Copy link
Contributor

I can confirm @mmsqe's finding. The Go commit that triggers the difference is golang/go@2719b1a. The underlying issue is that snapshots.StreamWriter double-closes the zlib writer, which results in the trailer written twice. For posterity,

// Close implements io.Closer interface
func (sw *StreamWriter) Close() error {
	if err := sw.protoWriter.Close(); err != nil {
		sw.chunkWriter.CloseWithError(err)
		return err
	}
	if err := sw.zWriter.Close(); err != nil {
		sw.chunkWriter.CloseWithError(err)
		return err
	}
	if err := sw.bufWriter.Flush(); err != nil {
		sw.chunkWriter.CloseWithError(err)
		return err
	}
	return sw.chunkWriter.Close()
}

Note that sw.protoWriter.Close() closes the underlying writer,

func (this *varintWriter) Close() error {
	if closer, ok := this.w.(io.Closer); ok {
		return closer.Close()
	}
	return nil
}

too.

A double-close before Go 1.19 would result in the compress/flate.compressor.close being called twice; in Go 1.19 the double close is silently ignored.

In any case, this issue is fixed as a side-effect of #13371.

@julienrbrt
Copy link
Member

julienrbrt commented Sep 24, 2022

Thanks for the explanation!

In any case, this issue is fixed as a side-effect of #13371.

TestManager_Take (/snapshots) and TestMultistoreSnapshot_Checksum/Format_1 (store/rootmulti) still fail.
Seems like for the same reason you've explained above.

@mmsqe
Copy link
Contributor

mmsqe commented Sep 25, 2022

TestManager_Take (/snapshots) and TestMultistoreSnapshot_Checksum/Format_1 (store/rootmulti) still fail. Seems like for the same reason you've explained above.

yes, but data written in 1.18 still can be decompressed correctly in 1.19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants