Speed up ExpireSnapshotAction Test by Reducing Shuffle Paralleism

[RussellSpitzer]

Because we use LocalIterator in ExpireSnapshotAction, every partition
runs it's own spark job, almost all of which are completely empty. This
leads to a lot of overhead which we don't need in the Test Suite. Setting
shuffle parallelism to 1 (from 200) greatly reduces the test runtime.

[rdblue]

Thanks! That looks much better.

[rdblue]

@RussellSpitzer, @aokolnychyi, if using the local iterator causes a job per task to be submitted to Spark, should we avoid using it?

If every file to delete takes up 500 bytes in memory, then the driver can hold 4 million files in 2GB. That seems reasonable to me, so we may be over-optimizing by using the iterator instead of just collecting the data back.

[RussellSpitzer]

That was my original thought, but I think we wanted to air on the side of caution. I think it would be fine if we just made it configurable. Add a switch for "lazy" execution?

…

On Thu, Aug 20, 2020 at 6:45 PM Ryan Blue ***@***.***> wrote: @RussellSpitzer <https://github.com/RussellSpitzer>, @aokolnychyi <https://github.com/aokolnychyi>, if using the local iterator causes a job per task to be submitted to Spark, should we avoid using it? If every file to delete takes up 500 bytes in memory, then the driver can hold 4 million files in 2GB. That seems reasonable to me, so we may be over-optimizing by using the iterator instead of just collecting the data back. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1362 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADE2YJS5YZZKAHDXL4ZLSDSBWYQ3ANCNFSM4QGVJG7Q> .

[rdblue]

That sounds good to me!

[RussellSpitzer]

Another idea is that we could just move all the deletes to the executors ... this would remove the need to configure something and all driver side work entirely. The only downsides I see to that are * we have to make a Serializable Consumer class for deletes (not a big deal) * we have to redo all the tests that use "Collection::add" to use an accumulator ... this should work * All logging about delete failures would be in executor logs so harder to actually see

…

On Thu, Aug 20, 2020 at 6:58 PM Ryan Blue ***@***.***> wrote: That sounds good to me! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1362 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADE2YKLRGNKVC2V7HZ7TQLSBW2B3ANCNFSM4QGVJG7Q> .