venkata91 commented on pull request #33034: URL: https://github.com/apache/spark/pull/33034#issuecomment-884387012
> > After having offline discussions with @mridulm , we decided not to break this PR in to 2. Will fix one of the pending change and remove the WIP tag. > > @venkata91 Could you please provide a reason for not breaking this PR in multiple parts. This PR is touching a lot files both on the client and server side. It changes the protocols. On the client, this again touches the driver side, the push side, and the fetch side. > We have broken changes in the past for similar sized changes to make the review easy and reduces the introduction of bugs. So why for this one we are making the exception? > cc. @mridulm @Victsm @Ngone51 There are couple of reasons: 1. Splitting the PR into 2 is not easy as well as clean - there are few classes which are dependent on both client and server side for eg: `MergeStatuses, FinalizeShuffleMerge` and if I start adding one of them to either client side changes or server side changes then the dependencies slowly expand and becomes hard to separate them into 2 clean units given now we have implementation also in place. 2. Given the timeline of RC, @mridulm feels if we break this in to 2 and 1 of them gets in but the other don't then that would be a problem. May be we can time it out so that both of them are close to completion and try to merge them both together. Any suggestions? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
