[kudu-CR] build: Move fake XML file generation to run-test.sh

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8984 )

Change subject: build: Move fake XML file generation to run-test.sh
..


Patch Set 3: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/8984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa89c806039a96ac0a6b7262ded74f70f49f87ac
Gerrit-Change-Number: 8984
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 16 Jan 2018 19:30:54 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#14).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output. This helps when triaging
issues in jenkins and other tools that rely on junit output. I included
all of the output, since it's difficult to know in advance which parts
will be useful. We may want to keep a close eye on Jenkins to see if
this results in too much output and we might decide to limit this to
just failures at the cost of some increase in parsing complexity.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_gtest_stdout_to_junit_xml.py
M build-support/jenkins/build-and-test.sh
2 files changed, 129 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/14
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 14
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#12).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output. This helps when triaging
issues in jenkins and other tools that rely on junit output. I included
all of the output, since it's difficult to know in advance which parts
will be useful. We may want to keep a close eye on Jenkins to see if
this results in too much output and we might decide to limit this to
just failures at the cost of some increase in parsing complexity.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 154 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/12
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 12
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#11).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output. This helps when triaging
issues in jenkins and other tools that rely on junit output. I included
all of the output, since it's difficult to know in advance which parts
will be useful. We may want to keep a close eye on Jenkins to see if
this results in too much output and we could decide to limit this to
just failures at the cost of some increase in parsing complexity.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 154 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/11
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 11
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8757 )

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..


Patch Set 10:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py
File build-support/jenkins/add_std_out_to_junit.py:

http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@36
PS7, Line 36: # Matches illegal characters as of the XML 1.0 spec.
> In your comment, please explain why we need this. Also, punctuation here an
Done


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@39
PS7, Line 39: y unhel
> This is a very mechanical name for a variable, almost as unhelpful as namin
Done


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@40
PS7, Line 40: # so we need to strip these characters out from the gtest output 
before
> Add docstring here and for process_logs.
Done


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@46
PS7, Line 46:  Given a suite name and test name, will walk the juni tree to 
find the
> We like to keep our code comments clean, please start with uppercase and en
Done


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@49
PS7, Line 49: here.
> nit: Mind using "stdout" instead of system-out? I think stdout will be very
system-out is what's mentioned in the junit jenkins schema: 
https://github.com/junit-team/junit5/blob/master/platform-tests/src/test/resources/jenkins-junit.xsd#L70
 (That may or may not be authoritative, but it was the best I could find.)


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@63
PS7, Line 63: child.text = ''.join(system_out_node_text_lines)
> Mind adding a comment here explaining what this loop is trying to accomplis
Done


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@64
PS7, Line 64:
> style nit: In comments, capitalize your sentences and end the comment with
Done


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@68
PS7, Line 68: obs for *.gz ands *. xml l
> what does < mean here? do you mean len(gz_filename) < len(xml_filename) ?
I added a comment for that. Basically, since they are sorted lexicographically, 
I'm using it to keep two running pointers between sorted lists to compare them 
one at a time, the same way you would for combining two sorted lists.


http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@69
PS7, Line 69: lphabetically. I use t
> nit: prefer the idiom gz_index += 1
Done



--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 10
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 13 Dec 2017 23:25:13 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#10).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output. This helps when triaging
issues in jenkins and other tools that rely on junit output. I included
all of the output, since it's difficult to know in advance which parts
will be useful. We may want to keep a close eye on Jenkins to see if
this results in too much output and we could decide to limit this to
just failures at the cost of some increase in parsing complexity.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 154 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/10
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 10
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#9).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output. This helps when triaging
issues in jenkins and other tools that rely on junit output. I included
all of the output, since it's difficult to know in advance which parts
will be useful. We may want to keep a close eye on Jenkins to see if
this results in too much output and we could decide to limit this to
just failures at the cost of some increase in parsing complexity.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 154 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/9
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 9
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#8).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output. This helps when triaging
issues in jenkins and other tools that rely on junit output. I included
all of the output, since it's difficult to know in advance which parts
will be useful. We may want to keep a close eye on Jenkins to see if
this results in too much output and we could decide to limit this to
just failures at the cost of some increase in parsing complexity.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 154 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/8
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 8
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#7).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 119 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/7
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 7
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#6).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 115 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/6
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 6
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#5).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 115 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/5
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 5
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#4).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 116 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/4
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 4
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#3).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 115 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/3
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 3
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8757

to look at the new patch set (#2).

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 112 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/2
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 2
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

[Edward Fancher (Code Review)]

Edward Fancher has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8757


Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..

KUDU-2216. Post process gtest generated xml to include the output from
the *.txt files

The gtest junit output doesn't include the output from the gtest run.
This adds a simple post process parser to pull that information from the
gtest result and put it in the junit output.

Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
---
A build-support/jenkins/add_std_out_to_junit.py
M build-support/jenkins/build-and-test.sh
2 files changed, 112 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/1
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 1
Gerrit-Owner: Edward Fancher 

[kudu-CR] KUDU-2033 (part 2). Add test for Java client failover support.

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2033 (part 2). Add test for Java client failover support.
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7722/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java:

PS1, Line 58: blic void testMu
> Move this closer to the point of the usage.
Done


PS1, Line 60: 
> Is it crucial to have that suffix?  I would expect it's not, so consider dr
Done


PS1, Line 61:  getBas
> nit: as the builder is not needed in the code below, consider not creating 
Done


Line 85: }
> Consider dropping the table in the very end, making sure it can be successf
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I228e08429d952f0f5b2657e0b8481366c5c572a4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 2). Add test for Java client failover support.

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7722

to look at the new patch set (#2).

Change subject: KUDU-2033 (part 2). Add test for Java client failover support.
..

KUDU-2033 (part 2). Add test for Java client failover support.

New test which validates that that the java client correctly gets
metadata updates after the Master leader is killed.

Change-Id: I228e08429d952f0f5b2657e0b8481366c5c572a4
---
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
1 file changed, 90 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/7722/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228e08429d952f0f5b2657e0b8481366c5c572a4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2033 (part 2). Add test for Java client failover support.

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7722

Change subject: KUDU-2033 (part 2). Add test for Java client failover support.
..

KUDU-2033 (part 2). Add test for Java client failover support.

New test which validates that that the java client correctly gets
metadata updates after the Master leader is killed.

Change-Id: I228e08429d952f0f5b2657e0b8481366c5c572a4
---
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
1 file changed, 87 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/7722/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I228e08429d952f0f5b2657e0b8481366c5c572a4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 

[kudu-CR] KUDU-871. Support tombstoned voting

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-871. Support tombstoned voting
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6960/6/src/kudu/integration-tests/tombstoned_voting-itest.cc
File src/kudu/integration-tests/tombstoned_voting-itest.cc:

Line 48: 
Replicated masters can have tombstoned replicas too, is that right? Does the 
tombstone voting apply to them? If so, would it be worthwhile to also have a 
test that includes tombstoned voting for a master tablet replica?


-- 
To view, visit http://gerrit.cloudera.org:8080/6960
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia19d75b185299443b27f41e468bbae20065e7570
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7456/7/java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java
File java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java:

PS7, Line 30: long deadlineNanos = System.nanoTime() + timeoutMillis * 
100;
: boolean success;
: 
: do {
:   success = expression.get();
:   if (success) break;
:  
> How about using a do-while loop so you only have to call .get() once in the
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7456

to look at the new patch set (#8).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
A java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java
3 files changed, 137 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/8
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..


Patch Set 6:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/7456/5/java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java:

PS5, Line 35: lon
> it's typical to use a long for millisecond time values in Java
Done


PS5, Line 64: NUM_ITERAT
> still named OUTER_LOOP, see comments on rev 4
Done


PS5, Line 71: DEFAU
> Better to not use magic numbers. For consistency with the rest of the clien
Done


PS5, Line 89: yet
> no need to say "yet" here, RYW will always be optional
Done


PS5, Line 90: DEFAU
> DEFAULT_SLEEP
Done


PS5, Line 93: DEFAU
> use DEFAULT_SLEEP here and elsewhere
Done


http://gerrit.cloudera.org:8080/#/c/7456/5/java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java
File java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java:

PS5, Line 27: 
> This is a standard Java thing and not required to be mentioned here
Done


Line 28:   public static void assertEventuallyTrue(String description, 
BooleanExpression expression,
> No need to give credit for this. Flume is ASL 2.0 which doesn't require att
Done


PS5, Line 36: 
> would be nice to avoid calling 'get()' twice, in case it's expensive. For e
Better?


-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7456

to look at the new patch set (#7).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
A java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java
3 files changed, 136 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/7
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7456

to look at the new patch set (#6).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
A java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java
3 files changed, 136 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..


Patch Set 5:

(22 comments)

http://gerrit.cloudera.org:8080/#/c/7456/4/java/kudu-client/src/test/java/org/apache/kudu/client/AssertHelpers.java
File java/kudu-client/src/test/java/org/apache/kudu/client/AssertHelpers.java:

Line 1
> Needs ASL 2.0 license header at the top of the file
Done


PS4, Line 2: 
> Seems more appropriate to put these in the "util" package instead of in "cl
Done


Line 4
> don't skip 2 blank lines, only 1
Done


PS4, Line 8: 
> nit: rename to BooleanExpression.
Done


Line 16
> style: it is not required, but to be consistent with the rest of the Kudu c
Done


Line 17
> style: indentation is off by 1. However I would suggest merging this line w
Done


http://gerrit.cloudera.org:8080/#/c/7456/4/java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java:

Line 31:   public static void setUpBeforeClass() throws Exception {
> style: don't leave multiple blank lines, only 1
Done


PS4, Line 34: 
> style: leave space before curly brace
Done


PS4, Line 34: 
> perhaps rename this to waitUntilRowCount() and allow the user to pass in ti
Done


PS4, Line 34: 
> nit: rowCount
Done


Line 35:   private void waitUntilRowCount(final KuduTable table, final int 
rowCount, int timeoutMs)
> specify "final int count" in the method signature instead
Done


Line 36:   throws Exception {
> No need to do this, specify "final KuduTable table" in the method signature
Done


Line 38: new BooleanExpression() {
> use "import AssertHelpers.BooleanPredicate" above to shorten this.
Done


Line 39:   @Override
> The indentation is off in this whole method (it should be 2 char indent, no
Ok, I redid it with intellij's settings. Does it seem correct now?


PS4, Line 45:  }, 
> Allow user to pass this in, however 5 sec is too short in extreme cases. Us
Done


Line 46:   }
> style: unnecessary blank line
Done


Line 48:   /**
> style: leave a blank space between functions
Done


PS4, Line 50: restarts t
> grammar: restarts
Done


PS4, Line 65: TOTAL_ROWS
> nit: OUTER_LOOP is a very mechanical name, and not really helpful for under
Done


PS4, Line 68: teBasicSch
> I think we can just use INNER_ROWS for this one, too. Maybe just get rid of
Done


Line 72: 
> Add comment that we have to potentially retry when scanning, which is why w
Done


Line 76:   assertEquals(1, tablets.size());
> I think this would be a little less awkward as two steps:
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7456

to look at the new patch set (#5).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
A java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java
3 files changed, 134 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7456/2/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java:

PS2, Line 351: int kil
> Can we return an int instead of an Integer here and below?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7456/2//COMMIT_MSG
Commit Message:

Line 7: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
> nit: Avoid period at end of subject line in a commit message
Fixed.


http://gerrit.cloudera.org:8080/#/c/7456/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestLeaderFailover.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestLeaderFailover.java:

Line 42:* This test writes 3 rows, kills the leader, then tries to write 
another 3 rows. Finally it
> nit: line length should be <= 100 chars
Done.


Line 49: KuduSession session = syncClient.newSession();
> Can we keep the existing simple test and add an additional test that uses t
Done


Line 51:   session.apply(createBasicSchemaInsert(table, i));
> style: use final and all caps for constants
Done


Line 67: scanner = client.newScannerBuilder(table).build();
> I find this logic a bit difficult to follow. I'd suggest keeping a counter 
Done


Line 77
> these assertions should probably be assert eventually
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7456

to look at the new patch set (#4).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
A java/kudu-client/src/test/java/org/apache/kudu/client/AssertHelpers.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
3 files changed, 124 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7456

to look at the new patch set (#3).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
A java/kudu-client/src/test/java/org/apache/kudu/client/AssertHelpers.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java
3 files changed, 124 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover.

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new patch set (#2).

Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to 
TestLeaderFailover.
..

KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover.

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestLeaderFailover.java
2 files changed, 29 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2033. Add a 'torture' scenario to verify Java client's behavior during fail-over

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7456

Change subject: KUDU-2033. Add a 'torture' scenario to verify Java client's 
behavior during fail-over
..

KUDU-2033. Add a 'torture' scenario to verify Java client's behavior
during fail-over

TestLeaderFailover is made more robust by adding a write loop in which
the tablet leader is killed and restarted. The main idea is to check
that the writes complete and can be read after the leader is killed.
Done in a loop so we can be sure that we stay stable through multiple
stops and restarts of the leader tablet.

Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
---
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestLeaderFailover.java
2 files changed, 29 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7456/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 

[kudu-CR] [net util] fix reporting getaddrinfo() error

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: [net_util] fix reporting getaddrinfo() error
..


Patch Set 2: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7368
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d5ba87b9ea050724367610461bae22c9471a103
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers

[Edward Fancher (Code Review)]

Hello Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7252

to look at the new patch set (#8).

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers
..

KUDU-1932. Run at least one tablet-level test against all block managers

Adding current block managers (file and log) to get extra coverage of
block managers. The file block manager especially has some thin coverage
since the log manager is the default on linux machines, such as
dist-test. Additionally, the file block manager is required on macs, so
it's easy for someone working just on a linux machine to break developers who
are using a mac to build and test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 42 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/8
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers

[Edward Fancher (Code Review)]

Hello Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7252

to look at the new patch set (#7).

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers
..

KUDU-1932. Run at least one tablet-level test against all block managers

Adding current block managers (file and log) to get extra coverage of
block managers. The file block manager especially has some thin coverage
since the log manager is the default on linux machines, such as
dist-test. Additionally, the file block manager is required on macs, so
it's easy for someone working just on a linux machine to break developers who
are using a mac to build and test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 42 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/7
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers
..


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7252/5//COMMIT_MSG
Commit Message:

PS5, Line 8: 
> This is a good one too: https://chris.beams.io/posts/git-commit/
Thanks. Can't beat a blog where they use xkcd references. :)


http://gerrit.cloudera.org:8080/#/c/7252/6/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

Line 79:   vector extra_tserver_flags_ = {};
> nit: not a big deal but typically leave a blank line between your method de
Done


PS6, Line 151:  
> nit: not your fault, but since you need to go back and address the Tidy Bot
Done


Line 189:   
extra_tserver_flags_with_crash.push_back("--fault_crash_during_log_replay=0.05");
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


Line 189:   
extra_tserver_flags_with_crash.push_back("--fault_crash_during_log_replay=0.05");
> emplace_back() is preferred when constructing an object. In this case, we'r
Yes. I think it also has stronger checks. It was giving me a runtime error when 
I tried to use it the way I would with a python style push, which allows you to 
concatenate lists. I only got an error at runtime with push_back.


Line 237:   extra_tserver_flags_.push_back("--log_segment_size_mb=1");
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


Line 238:   
extra_tserver_flags_.push_back("--log_compression_codec=NO_COMPRESSION");
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..


Patch Set 5:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/7252/5//COMMIT_MSG
Commit Message:

PS5, Line 8: .
> nit: don't end your commit message with a period and don't wrap the first l
Done


PS5, Line 8: .
> Just for reference, there is useful info on writing commit messages: https:
Thanks!


Line 10: Decided to do it against ts_recovery-itest instead. Converted all the
> nit: this "decided ... instead" only makes sense with recent memory of the 
Done


Line 13: Kudu969Test.
> some style notes:
Replaced with 
Adding current block managers (file and log) to get extra coverage of block 
managers. The file block manager especially has some thin coverage since the 
log manager is the default on linux machines. Additionally, the file block 
manager is required on macs, so it's easy for someone working just on a linux 
machine to break developers who are using a mac to build and test.


http://gerrit.cloudera.org:8080/#/c/7252/5/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

PS5, Line 76:   vector extra_tserver_flags = 
> should be suffixed with '_'
Done.


Line 79:   TsRecoveryITest();
> style nit: since this is just a one-line constructor, inline it
Done.


PS5, Line 92: block_mgr_flags
> nit: should be kBlockManagerFlags per google style guide
Done


Line 111:   // vector extra_tserver_flags = {};
> what's this?
Oops. Removed.


Line 195:   
extra_tserver_flags_with_crash.push_back({"--fault_crash_during_log_replay=0.05"});
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


Line 243:   extra_tserver_flags.push_back({"--log_segment_size_mb=1",
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7252

to look at the new patch set (#6).

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers
..

KUDU-1932. Run at least one tablet-level test against all block managers

Adding current block managers (file and log) to get extra coverage of
block managers. The file block manager especially has some thin coverage
since the log manager is the default on linux machines, such as
dist-test. Additionally, the file block manager is required on macs, so
it's easy for someone working just on a linux machine to break developers who
are using a mac to build and test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 42 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7252

to look at the new patch set (#4).

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 49 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7252

to look at the new patch set (#3).

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 51 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7252

to look at the new patch set (#2).

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 38 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1932. Run at least one tablet-level test against all block managers.

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7252

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
..

KUDU-1932. Run at least one tablet-level test against all block
managers.

Decided to do it against ts_recovery-itest instead. Converted all the
TEST_F into TEST_P's. And added a INSTANTIATE_TEST_CASE_P to
provide the extra parameters to TSRecoveryITest. Did not alter
Kudu969Test.

Dist test results:
http://dist-test.cloudera.org/job?job_id=efan.1497991165.6690

Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 38 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/7252/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 

[kudu-CR] KUDU-2004. Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2004. Undefined behavior in TlsSocket::Writev()
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7141/10//COMMIT_MSG
Commit Message:

PS10, Line 7: .
> nit: Mind using a period instead of a colon here per JD's comment?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2004. Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2004. Undefined behavior in TlsSocket::Writev()
..


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7141/8//COMMIT_MSG
Commit Message:

Line 7: KUDU-2004. Undefined behavior in TlsSocket::Writev()
> err only one space after the period.
Done


Line 7: KUDU-2004. Undefined behavior in TlsSocket::Writev()
> When there's a corresponding jira we prefix the first line like this:
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2004. Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#11).

Change subject: KUDU-2004. Undefined behavior in TlsSocket::Writev()
..

KUDU-2004. Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 10 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/11
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2004: Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: KUDU-2004: Undefined behavior in TlsSocket::Writev()
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7141/9/src/kudu/util/net/socket.h
File src/kudu/util/net/socket.h:

PS9, Line 123:   // Write up to 'amt' bytes from 'buf' to the socket. The 
number of bytes
 :   // actually written will be stored in 'nwritten'. If an error 
is returned,
> Did you see my suggestion in CR 4 for this comment?
Sorry, I made a mistake in translating the line numbers from tls_socket.h to 
socket.h and I fixed the wrong comment. Should be better now.


-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2004: Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#10).

Change subject: KUDU-2004: Undefined behavior in TlsSocket::Writev()
..

KUDU-2004: Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 10 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/10
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2004: Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#9).

Change subject: KUDU-2004: Undefined behavior in TlsSocket::Writev()
..

KUDU-2004: Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/9
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: Undefined behavior in TlsSocket::Writev()
..


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7141/4/src/kudu/security/tls_socket.cc
File src/kudu/security/tls_socket.cc:

PS4, Line 80: if (!write_status.ok()) break;
: 
> I think this comment can be simplified by saying something like:
Removed this per Adar.


PS4, Line 84: if (*nwritten < frame_size) break;
> How about:
Removed this per Adar.


http://gerrit.cloudera.org:8080/#/c/7141/4/src/kudu/security/tls_socket.h
File src/kudu/security/tls_socket.h:

Line 35:   Status Write(const uint8_t *buf, int32_t amt, int32_t *nwritten) 
override WARN_UNUSED_RESULT;
> +1 for moving to class declaration for Socket.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#8).

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/8
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#7).

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 10 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/7
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: Undefined behavior in TlsSocket::Writev()
..


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7141/5/src/kudu/security/tls_socket.cc
File src/kudu/security/tls_socket.cc:

Line 80: if (!write_status.ok()) break;
> To be honest I don't think this comment is necessary either; the "Status s 
Done


Line 81: 
> This comment duplicates the one on L78.
Done


http://gerrit.cloudera.org:8080/#/c/7141/5/src/kudu/util/net/socket.h
File src/kudu/util/net/socket.h:

Line 124:   // If amt == nwritten == 0, this was a no-op.
> Don't you mean "this was a no-op"? That is, if nwritten == 0 after the call
is => was.


-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#6).

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 10 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: Undefined behavior in TlsSocket::Writev()
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7141/4/src/kudu/security/tls_socket.h
File src/kudu/security/tls_socket.h:

Line 35:   Status Write(const uint8_t *buf, int32_t amt, int32_t *nwritten) 
override WARN_UNUSED_RESULT;
> Could you instead add these docs to the parent class in socket.h, they shou
Done


Line 44: 
> The nwritten index isn't valid on the iov array.  I'd just simplify and say
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#5).

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/util/net/socket.h
2 files changed, 12 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#3).

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/security/tls_socket.h
2 files changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7141

to look at the new patch set (#2).

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/security/tls_socket.h
2 files changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Undefined behavior in TlsSocket::Writev()

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7141

Change subject: Undefined behavior in TlsSocket::Writev()
..

Undefined behavior in TlsSocket::Writev()

TlsSocket::Writev() was attempting to use the value of nwritten from
TlsSocket::Write(), but in the case of an error that value was never
set or initialized. A simple check to make sure the result from
TlsSocket::Write() wasn't an error was added, otherwise we break out of
the write loop to cleanup and return the error (thus skipping the line
that uses nwritten)

Dist job result from before the fix:
http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151

Dist job result from after the fix:
http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311

Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
---
M src/kudu/security/tls_socket.cc
M src/kudu/security/tls_socket.h
2 files changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/7141/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 

[kudu-CR] Fix flaky test TestRestartWithOrphanedReplicates

[Edward Fancher (Code Review)]

Hello Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/6976

to look at the new patch set (#4).

Change subject: Fix flaky test TestRestartWithOrphanedReplicates
..

Fix flaky test TestRestartWithOrphanedReplicates

Looks like the test was setting a fault injection flag before the setup
which was causing the setup to fail. Moving the flag setting to after
the setup, but before Start seems to have done the trick.
We were getting about 1 failure per 1000 runs before this change,
0 per 1000 after.
Dist test job after patch:

http://dist-test.cloudera.org/job?job_id=efan.1495643022.19373

Dist test job before patch with failures:
http://dist-test.cloudera.org/job?job_id=efan.1495149936.30555

Example from a failure log:

I0518 23:26:00.447599  2346 master_service.cc:195] Signed X509 certificate for 
tserver {username='slave'} at 127.9.10.0:34667
W0518 23:26:00.448045  2529 fault_injection.cc:38] FAULT INJECTION ENABLED!
W0518 23:26:00.448065  2529 fault_injection.cc:39] THIS SERVER MAY CRASH!
E0518 23:26:00.448072  2529 fault_injection.cc:54] Injecting fault: 
FLAGS_fault_crash_before_append_commit (process will exit)
I0518 23:26:00.448150  2510 heartbeater.cc:380] Master 127.0.0.1:38357 was 
elected leader, sending a full tablet report...
W0518 23:26:00.450608  2331 connection.cc:462] server connection from 
127.9.10.0:34667 recv error: Network error: failed to read from TLS socket: 
Connection reset by peer (error 104)
W0518 23:26:00.450630  2319 connection.cc:462] client connection to 
127.9.10.0:51332 recv error: Network error: failed to read from TLS socket: 
Connection reset by peer (error 104)
W0518 23:26:00.450822  2331 connection.cc:462] client connection to 
127.9.10.0:51332 recv error: Network error: failed to read from TLS socket: 
Connection reset by peer (error 104)
F0518 23:26:30.315480  2314 test_workload.cc:266] Timed out: Timed out waiting 
for Table Creation
*** Check failure stack trace: ***
@ 0x7fb089e3b2fd  google::LogMessage::Fail() at ??:0
@ 0x7fb089e3d1bd  google::LogMessage::SendToLog() at ??:0
@ 0x7fb089e3ae39  google::LogMessage::Flush() at ??:0
@ 0x7fb089e3dc5f  google::LogMessageFatal::~LogMessageFatal() at ??:0
@ 0x7fb0943b7492  kudu::TestWorkload::Setup() at ??:0
@   0x40e68e  
kudu::TsRecoveryITest_TestRestartWithOrphanedReplicates_Test::TestBody() at 
/home/efan/src/kudu/build/release/../../src/kudu/integration-tests/ts_recovery-itest.cc:107
@ 0x7fb08ad35af8  
testing::internal::HandleExceptionsInMethodIfSupported<>() at ??:0
@ 0x7fb08ad29fc2  testing::Test::Run() at ??:0
@ 0x7fb08ad2a108  testing::TestInfo::Run() at ??:0
@ 0x7fb08ad2a1e5  testing::TestCase::Run() at ??:0
@ 0x7fb08ad2a4c8  testing::internal::UnitTestImpl::RunAllTests() at ??:0
@ 0x7fb08ad36008  
testing::internal::HandleExceptionsInMethodIfSupported<>() at ??:0
@ 0x7fb08ad2a7ad  testing::UnitTest::Run() at ??:0
@ 0x7fb09416566c  main at ??:0
@ 0x7fb088083f45  __libc_start_main at ??:0
@   0x40dfb9  (unknown) at ??:?

Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 6 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/6976/4
-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Fix flaky test TestRestartWithOrphanedReplicates

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: Fix flaky test TestRestartWithOrphanedReplicates
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6976/2/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

Line 97:   // 1. Setup
> I don't think this comment really adds any value to this test.
I'll remove it.


Line 108:   cluster_->SetFlag(cluster_->tablet_server(0),
> I think a better comment here would be:
Ok. I'll replace it.


Line 121:   // 3. Validate
> I think a better comment here would be:
Ok, I'll replace it.


-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Fix flaky test TestRestartWithOrphanedReplicates

[Edward Fancher (Code Review)]

Edward Fancher has posted comments on this change.

Change subject: Fix flaky test TestRestartWithOrphanedReplicates
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6976/1//COMMIT_MSG
Commit Message:

PS1, Line 7: Fix flaky test
> it would be nice to add name of the test here
Ok, will do.


PS1, Line 9: Looks like the test was setting a fault injection flag before the 
setup
   : which was causing the setup to fail
> It would be nice to add the error message that the failed setup output into
Ok, will do.


PS1, Line 12: 1 failure per 1000
> Is it possible to add a link to one of those 1-in-1K dist-test  failed runs
Yea, let me stash it on ve0518 and I can do a rerun.


-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Fix flaky test TestRestartWithOrphanedReplicates

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new patch set (#2).

Change subject: Fix flaky test TestRestartWithOrphanedReplicates
..

Fix flaky test TestRestartWithOrphanedReplicates

Looks like the test was setting a fault injection flag before the setup
which was causing the setup to fail. Moving the flag setting to after
the setup, but before Start seems to have done the trick.
We were getting about 1 failure per 1000 runs before this change,
0 per 1000 after.
Dist test job:

http://dist-test.cloudera.org/job?job_id=efan.1495643022.19373

Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 7 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/6976/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Fix flaky test

[Edward Fancher (Code Review)]

Edward Fancher has restored this change.

Change subject: Fix flaky test
..


Restored

-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: restore
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix flaky test

[Edward Fancher (Code Review)]

Edward Fancher has abandoned this change.

Change subject: Fix flaky test
..


Abandoned

-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix flaky test

[Edward Fancher (Code Review)]

Edward Fancher has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6976

Change subject: Fix flaky test
..

Fix flaky test

Looks like the test was setting a fault injection flag before the setup
which was causing the setup to fail. Moving the flag setting to after
the setup, but before Start seems to have done the trick.
We were getting about 1 failure per 1000 runs before this change,
0 per 1000 after.
Dist test job:

http://dist-test.cloudera.org/job?job_id=efan.1495643022.19373

Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 7 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/6976/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6976
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied9a55abd20841d350589ce56aa935ea1feece79
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Edward Fancher