release-2.0: importccl: Preserve '\r\n' during CSV import#28225
Merged
craig[bot] merged 2 commits intocockroachdb:release-2.0from Aug 6, 2018
Merged
release-2.0: importccl: Preserve '\r\n' during CSV import#28225craig[bot] merged 2 commits intocockroachdb:release-2.0from
craig[bot] merged 2 commits intocockroachdb:release-2.0from
Conversation
Member
|
This change is |
neeral
reviewed
Aug 3, 2018
Contributor
neeral
left a comment
There was a problem hiding this comment.
Note: import_stmt_test.go in master was renamed from csv_test.go in branch release-2.0 -- the unit test from #28181 hasn't made it into this PR.
Reviewable status:
complete! 0 of 0 LGTMs obtained
bdarnell
approved these changes
Aug 3, 2018
Contributor
bdarnell
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/util/encoding/csv/example_test.go, line 3 at r1 (raw file):
// Copyright 2015 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file.
The copyright headers in all copied files need to be updated to point to the license in licenses/BSD-golang.txt (see syncutil for an example)
See cockroachdb#25344. It appears this is being caused by the behaviour of Golang's encoding/csv library, which folds \r\n into \n when reading. This was fixed in golang/go#21201 but then reverted golang/go#22746. It appears based on that second issue that Go is unlikely to change that behavior. Check in the stdlib `encoding/csv` into `pkg/util` with golang/go#22746 reverted. Release note: `\r\n` characters in CSV files were silently converted into `\n`. This causes imported data to be different. This is now fixed.
Forgot to update these when vendoring the stdlib package. Also switch the Example_ tests to test this package instead of stdlib. Release note: none.
Contributor
Author
benesch
approved these changes
Aug 6, 2018
Contributor
Author
|
now just need to run the flaky 2.0 test gauntlet |
Contributor
Author
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Aug 6, 2018
27868: backport-2.0: storage: prevent unbounded raft log growth without quorum r=nvanbenschoten a=nvanbenschoten Backport 2/2 commits from #27774. /cc @cockroachdb/release --- Fixes #27772. This change adds safeguards to prevent cases where a raft log would grow without bound during loss of quorum scenarios. It also adds a new test that demonstrates that the raft log does not grow without bound in these cases. There are two cases that need to be handled to prevent the unbounded raft log growth observed in #27772. 1. When the leader proposes a command and cannot establish a quorum. In this case, we know the leader has the entry in its log, so there's no need to refresh it with `reasonTicks`. To avoid this, we no longer use `refreshTicks` as a leader. 2. When a follower proposes a command that is forwarded to the leader who cannot establish a quorum. In this case, the follower can't be sure (currently) that the leader got the proposal, so it needs to refresh using `reasonTicks`. However, the leader now detects duplicate forwarded proposals and avoids appending redundant entries to its log. It does so by maintaining a set of in-flight forwarded proposals that it has received during its term as leader. This set is reset after every leadership change. Both of these cases are tested against in the new TestLogGrowthWhenRefreshingPendingCommands. Without both of the safeguards introduced in this commit, the test fails. Release note (bug fix): Prevent loss of quorum situations from allowing unbounded growth of a Range's Raft log. 28225: release-2.0: importccl: Preserve '\r\n' during CSV import r=dt a=dt Backport 1/1 commits from #28181. /cc @cockroachdb/release --- See #25344. Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: neeral <neeral@users.noreply.github.com> Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Contributor
Build succeeded |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport 1/1 commits from #28181.
/cc @cockroachdb/release
See #25344.