Skip to content

Update the default configs for spread flush and bigchunks #2207

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

Merged
merged 5 commits into from
Mar 9, 2020

Conversation

tomwilkie
Copy link
Contributor

What this PR does:

  • Make spread flush the default using the example single binary config (not the flag)
  • Make bigchunk the default encoding in the code

Which issue(s) this PR fixes:
Fixes #2202

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

👍

@gouthamve
Copy link
Contributor

Hrm, tests are failing :(

@tomwilkie
Copy link
Contributor Author

Hrm, tests are failing :(

Will fix this up at the weekend.

Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

I found it slightly confusing that one change is in the code and one is in an example config, but other than some spelling improvements it's fine to merge.

@pracucci pracucci force-pushed the single-process-config branch from 6fedb2c to 0850ab0 Compare March 9, 2020 09:28
@pracucci
Copy link
Contributor

pracucci commented Mar 9, 2020

Rebased master to fix conflicts

@pracucci pracucci force-pushed the single-process-config branch from 0850ab0 to 869fe64 Compare March 9, 2020 10:36
Signed-off-by: Marco Pracucci <[email protected]>
@pracucci
Copy link
Contributor

pracucci commented Mar 9, 2020

In tests we do compare the chunk object. Switching to Bigchunk the comparison is not clean because of the appender pointer. In the commit 44ada47 I've used a trick to cleanup the expected chunk (the clean chunk has no appender internally). I'm not much happy with the trick I've done, but haven't come up with a better idea.

Copy link
Contributor

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

LGTM to this change and the change Marco made.

@pracucci pracucci merged commit d0ca826 into master Mar 9, 2020
@pracucci pracucci deleted the single-process-config branch March 9, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Make BigChunk the default encoding
4 participants