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

Release 0.4.17 #5267

Merged
merged 2 commits into from
Jul 24, 2018
Merged

Release 0.4.17 #5267

merged 2 commits into from
Jul 24, 2018

Conversation

Stebalien
Copy link
Member

This is a relatively small release to fix a performance regression I introduced.
Hopefully, it'll also get us into a fast release cadence.

The downside is that freezing due to an RC slows everything to a grind.

@Stebalien Stebalien requested a review from Kubuxu as a code owner July 20, 2018 20:28
@ghost ghost assigned Stebalien Jul 20, 2018
@ghost ghost added the status/in-progress In progress label Jul 20, 2018
@Stebalien
Copy link
Member Author

It may be worth it to merge #5117 first to get another feature in.

@Stebalien Stebalien added need/review Needs a review status/in-progress In progress need/community-input Needs input from the wider community and removed status/in-progress In progress labels Jul 20, 2018
@Stebalien
Copy link
Member Author

We should probably change this to an RC but this PR can still be reviewed as-is.

@Stebalien
Copy link
Member Author

Also needs the versions updated.

@magik6k
Copy link
Member

magik6k commented Jul 21, 2018

Probably want to fix #5270 first

@Stebalien
Copy link
Member Author

Definitely. And we'll also want to cut an RC.

@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Jul 21, 2018
whyrusleeping
whyrusleeping reviewed Jul 23, 2018
View reviewed changes
CHANGELOG.md Outdated
## 0.4.16
## 0.4.17 2018-07-20

Ipfs 0.4.17 is a quick release to fix a major performance regression in bitswap.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting that the regression only affects go to javascript transfers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Affects all, just mostly JS.

whyrusleeping
whyrusleeping reviewed Jul 23, 2018
View reviewed changes
CHANGELOG.md Outdated
URLs on-demand instead of storing them in the local datastore.

Users may also notice reduced CPU usage on IPFS nodes participating in an IPFS
cluster.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

" due to .... "

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind. This release adds a feature that'll help cluster but I'm not sure if cluster uses it yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fast repo/stat? yes it's used. But I think it reduces disk IO, not cpu..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. It reduces both, IIRC. However, in retrospect, that's probably more of a cluster release note.

whyrusleeping
whyrusleeping reviewed Jul 23, 2018
View reviewed changes
CHANGELOG.md Outdated

Documentation:

* improve documentation about the bloomfilter config options ([ipfs/go-ipfs#4924](https://github.com/ipfs/go-ipfs/pull/4924))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capitalize 'improve'

whyrusleeping
whyrusleeping reviewed Jul 23, 2018
View reviewed changes
CHANGELOG.md Outdated
@@ -1,6 +1,53 @@
# go-ipfs changelog

## 0.4.16
## 0.4.17 2018-07-20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be rc1

@kevina
Copy link
Contributor

kevina commented Jul 23, 2018

Note that by merging #5117 in without a migration any existing id hashes in the data store will now be impossible to remove by normal means. I don't think this is an issue but it is worth nothing. If it is enough of a concern I can create a migration just to remove empty id hashes, but I was hoping to combine that with the strict CID check.

@Stebalien
Copy link
Member Author

Note that by merging #5117 in without a migration any existing id hashes in the data store will now be impossible to remove by normal means. I don't think this is an issue but it is worth nothing. If it is enough of a concern I can create a migration just to remove empty id hashes, but I was hoping to combine that with the strict CID check.

Can't we add a migration to do this later, technically?

@kevina
Copy link
Contributor

kevina commented Jul 23, 2018 •
edited
Loading

Can't we add a migration to do this later, technically?

Yes. But for a brief period there will be hashes in the data store will now be impossible to remove by normal means until the migration is done. Again don't think this is an issue but wanted to mention it.

@Stebalien
Copy link
Member Author

Fair enough, I'm just saying that the user shouldn't even notice.

@kevina
Copy link
Contributor

kevina commented Jul 23, 2018

Fair enough, I'm just saying that the user shouldn't even notice.

Not unless they already have a large number of id hashes in the datastore, which is highly unlikely.

@Stebalien Stebalien force-pushed the release-0.4.17 branch 4 times, most recently from c21b028 to 6acf41a Compare July 24, 2018 00:57
@Stebalien Stebalien removed status/blocked Unable to be worked further until needs are met need/community-input Needs input from the wider community status/in-progress In progress labels Jul 24, 2018
@Stebalien
update release notes and version for 0.4.17-rc1
50f9401 
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@ghost ghost added the status/in-progress In progress label Jul 24, 2018
@Stebalien
release 0.4.17-rc1
3a085b7 
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien Stebalien added RFM and removed status/in-progress In progress need/review Needs a review labels Jul 24, 2018
@Stebalien
Copy link
Member Author

@whyrusleeping needs tag.

@whyrusleeping whyrusleeping merged commit f420578 into master Jul 24, 2018
@whyrusleeping whyrusleeping deleted the release-0.4.17 branch July 24, 2018 06:30
@whyrusleeping
Copy link
Member

@Stebalien tagged it!

magik6k
magik6k reviewed Jul 24, 2018
View reviewed changes
CHANGELOG.md

1. A major performance regression in bitswap (mostly affecting go-ipfs ->
js-ipfs transfers).
2. A bug resolving files in sharded directories with the gateway.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH that issue wasn't present in 0.4.16 - was introduced in #4672 which got merged after that release

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. Good point. We should fix that before the actual release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@magik6k magik6k magik6k left review comments

@hsanjuan hsanjuan hsanjuan left review comments

@whyrusleeping whyrusleeping whyrusleeping approved these changes

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

Assignees

@Stebalien Stebalien

Labels
RFM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Stebalien @magik6k @kevina @whyrusleeping @hsanjuan