spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nicholas Chammas <nicholas.cham...@gmail.com>
Subject Re: Is this a little bug in BlockTransferMessage ?
Date Tue, 09 Dec 2014 23:35:54 GMT
OK. That's concerning. Hopefully that's the only bug we'll dig up once we
run all the Java tests but who knows.

Patrick,

Shouldn't this be a release blocking bug for 1.2 (mostly just because it
has already been covered by a unit test)? Well, that, as well as any other
bugs that come up as we run these Java tests.

Nick

On Tue Dec 09 2014 at 6:32:53 PM Sean Owen <sowen@cloudera.com> wrote:

> I'm not so sure about SBT, but I'm looking at the output now and do
> not see things like JavaAPISuite being run. I see them compiled. That
> I'm not as sure how to fix. I think I have a solution for Maven on
> SPARK-4159.
>
> On Tue, Dec 9, 2014 at 11:30 PM, Nicholas Chammas
> <nicholas.chammas@gmail.com> wrote:
> > So all this time the tests that Jenkins has been running via Jenkins and
> SBT
> > + ScalaTest... those haven't been running any of the Java unit tests?
> >
> > SPARK-4159 only mentions Maven as a problem, but I'm wondering how these
> > tests got through Jenkins OK.
> >
> > On Tue Dec 09 2014 at 5:34:22 PM Sean Owen <sowen@cloudera.com> wrote:
> >>
> >> Yep, will do. The test does catch it -- it's just not being executed.
> >> I think I have a reasonable start on re-enabling surefire + Java tests
> >> for SPARK-4159.
> >>
> >> On Tue, Dec 9, 2014 at 10:30 PM, Aaron Davidson <aaron@databricks.com>
> >> wrote:
> >> > Oops, that does look like a bug. Strange that the
> >> > BlockTransferMessageSuite
> >> > did not catch this. "+1" sounds like the right solution, would you be
> >> > able
> >> > to submit a PR?
> >> >
> >> > On Tue, Dec 9, 2014 at 1:53 PM, Sean Owen <sowen@cloudera.com> wrote:
> >> >>
> >> >>
> >> >>
> >> >> https://github.com/apache/spark/blob/master/network/
> shuffle/src/main/java/org/apache/spark/network/shuffle/
> protocol/BlockTransferMessage.java#L70
> >> >>
> >> >> public byte[] toByteArray() {
> >> >>   ByteBuf buf = Unpooled.buffer(encodedLength());
> >> >>   buf.writeByte(type().id);
> >> >>   encode(buf);
> >> >>   assert buf.writableBytes() == 0 : "Writable bytes remain: " +
> >> >> buf.writableBytes();
> >> >>   return buf.array();
> >> >> }
> >> >>
> >> >> Running the Java tests at last might have turned up a little bug
> here,
> >> >> but wanted to check. This makes a buffer to hold enough bytes to
> >> >> encode the message. But it writes 1 byte, plus the message. This
> makes
> >> >> the buffer expand, and then does have nonzero capacity afterwards,
so
> >> >> the assert fails.
> >> >>
> >> >> So just needs a "+ 1" in the size?
> >> >
> >> >
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
> >> For additional commands, e-mail: dev-help@spark.apache.org
> >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message