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

msgpack dependencies #473

Closed
SimonRichardson opened this issue Sep 13, 2021 · 7 comments
Closed

msgpack dependencies #473

SimonRichardson opened this issue Sep 13, 2021 · 7 comments

Comments

@SimonRichardson
Copy link
Contributor

This is more of a potential problem that's up and coming, rather than a problem right now (msgpack v1.1.5 tag and mspack v0.5.5 as seen https://github.com/hashicorp/go-msgpack/tags). The upstream msgpack has changed how time was encoded/decoded and the workaround was provided.

I suspect either two options, decode the data with TimeNotBuiltin set and then encode it without, allowing a stable migration or it might be prudent to expose TimeNotBuiltin in the raft configuration (under another name LegacyEncoding or similar), so this can be changed for users with existing data.

@ncabatoff
Copy link
Contributor

Hi @SimonRichardson,

It looks like there's been a new release of hashicorp/go-msgpack, and presumably that maintains our fork's absention from the new time serialization. We'll update this library to make use of that 1.1.5 release.

@SimonRichardson
Copy link
Contributor Author

If you update raft, then existing deployments that utilize the 0.5.5 release that upgrades to 1.5.5 will hit the following error, unless a migration is applied.

failed to get last log at index 52: msgpack decode error [pos 13]: invalid length of bytes for decoding time - expecting 4 or 8 or 12, got 15

@ncabatoff
Copy link
Contributor

Ah, I think I misunderstood you. I thought you were talking about upgrading to newer hashicorp/go-msgpack versions that already exist. But your actual concern was: if hashicorp/go-msgpack were to pull in the upstream changes from ugorji/go, in particular ugorji/go@debb8e2, then any time.TIme values we serialized with older msgpack versions would be a problem. Is that right?

@SimonRichardson
Copy link
Contributor Author

Indeed.

@stale
Copy link

stale bot commented Jan 9, 2022

Hey there,
We wanted to check in on this request since it has been inactive for at least 90 days.
Have you reviewed the latest godocs?
If you think this is still an important issue in the latest version of the Raft library or
its documentation please feel let us know and we'll keep it open for investigation.
If there is still no activity on this request in 30 days, we will go ahead and close it.
Thank you!

@stale stale bot added the waiting-reply label Jan 9, 2022
@wallyworld
Copy link

This is still an issue that it would be good to see a fix for.

@ncabatoff
Copy link
Contributor

This has been addressed, see #577 (comment).

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

No branches or pull requests

3 participants