Thanks for the work, Sangjin! Please go ahead to complete rebase. About moving TestRPC from hadoop-common to hadoop-server-common, we were adding "testRPCOnCollectorNodeManagerProtocol" by reusing existing code in TestRPC which deal with server protocol so have to stay in hadoop-server-common now. There is also another option to separate this test case (added in YARN-2928 branch only) as a separated test class (like: TestServerRPC) with copy some test code from TestRPC, but it is not quite reasonable just for merge convenient.
Thanks, Junping? ________________________________ Thanks Li! Naga's also fine with d35d861. Junping, are you OK with completing the rebase as it stands? Sangjin On Mon, Oct 12, 2015 at 6:12 PM, Li Lu <[email protected]> wrote: > Thanks Sangjin for the work! I've tested our temporary fix on HBase UT > failures (bd5af9c) and it looks good to me. I'm not an expert in HTrace or > HDFS, but so far the fix works on our side. We may need to revert our > current fix after HDFS-9187 is officially done, though. For now I'm +1 on > the temp fix in YARN-2928 branch. > > Li Lu > > On Oct 12, 2015, at 18:02, Sangjin Lee <[email protected]<mailto: > [email protected]>> wrote: > > Hi folks, > > I have completed the rebase of YARN-2928 (this time cherry-picks really) to > the trunk as of last Saturday. I resolved 10 merge conflicts most of which > were minor. But I do want to call out a few of them, and would like you to > review how I resolved those conflicts before I make the rebase official. I > have just pushed this new branch ("*YARN-2928-rebase*") so you can take a > look at it. I'll swap the branches once we're satisfied. > > The following are those commits to review. I called out those who might be > best to review the merges. > > [3e3a8fe: Junping] > Trunk added a new use (in TestContainerResourceIncreaseRPC > < > https://github.com/apache/hadoop/blob/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/TestContainerResourceIncreaseRPC.java#L99 > >) > of a method (TestRPC.newContainerToken) in TestRPC which we moved from > yarn-common to yarn-server-common. I copied that method in > TestContainerResourceIncreaseRPC. We could reconsider whether we want to > move TestRPC from yarn-common to yarn-server-common. I don't recall the > details of the discussion, but was there a strong reason to move TestRPC > out of yarn-common? If trunk keeps creating new uses of this class, it > might be a problem. > > [d35d861: Naga] > Trunk added a new RM event type (app updated: YARN-4044 > < > https://github.com/apache/hadoop/commit/a9aafad12b1d2f67e55e09a6fa261d61789c9d7e > >). > I applied the same changes and moved code to > AbstractTimelineServicePublisher, TimelineServiceV1Publisher, and > TimelineServiceV2Publisher respectively. Naga, could you please confirm if > that new event is done right in the merge commit? > > [bd5af9c] > It turns out HDFS-9080 broke the HBase mini-cluster, which in turn broke > our HBase-based unit tests. This was caught by HDFS-9187 which has a patch. > The patch is not entirely correct (causes NPEs), and I applied a fixed > version of that patch to our branch to ensure our tests pass. Let me know > if you are OK with that. I don't think we can wait until HDFS-9187 gets > resolved. > > If you could take a look at these commits, and let me know +1/-1, I'll be > able to take the next steps. Thanks everyone! > > Regards, > Sangjin >?
