MSQ: Change default clusterStatisticsMergeMode to SEQUENTIAL.

[gianm]

This is an undocumented parameter that controls how cluster-by statistics are merged. In PARALLEL mode, statistics are gathered from workers all at once. In SEQUENTIAL mode, statistics are gathered time chunk by time chunk. This improves accuracy for jobs with many time chunks, and reduces memory usage.

The main downside of SEQUENTIAL is that it can take longer, but in most situations I've seen, PARALLEL is only really usable in cases where the sketches are small enough that SEQUENTIAL would also run relatively quickly. So it seems like SEQUENTIAL is a better default.

[cryptoe]

Segments cuts for sequential would be atleast equal to segment cuts in parallel mode. I have seen cases where the job runs 30% faster when changing modes from parallel to sequential when number of workers was 1000.

[cryptoe]

We should also change

• MSQTestBase#245 to PARALLEL_MERGE_CONTEXT since now the default has changed/

This can be done in another PR as well

[gianm]

We should also change

* MSQTestBase#245 to PARALLEL_MERGE_CONTEXT since now the default has changed/

This can be done in another PR as well

Ah, I'll do it in this patch, since it needs updates anyway due to one of the test cases failing. Currently, the test case MSQFaultsTest.testInsertCannotBeEmptyFault is timing out, I suppose because the sequential fetching logic doesn't handle the case where no workers have any time chunks. @cryptoe any idea why that might be happening? If not, I'll take a deeper look into it soon.

[cryptoe]

@gianm Quite possible we missed this case since we throw InsertCannotByEmptyFault only after getting the partition boundaries.

    if (isTimeBucketed && partitionBoundaries.equals(ClusterByPartitions.oneUniversalPartition())) {
            throw new MSQException(new InsertCannotBeEmptyFault(task.getDataSource()));
          } else {
            log.info("Query [%s] generating %d segments.", queryDef.getQueryId(), partitionBoundaries.size());
          }

The fix would be here WorkerSketcherFetcher#235 . Need to check if CompleteKeyStatisticsInformation is empty.

[gianm]

The fix would be here WorkerSketcherFetcher#235 . Need to check if CompleteKeyStatisticsInformation is empty.

TY, I added a block there that registers ClusterByStatisticsSnapshot.empty() for all workers if completeKeyStatisticsInformation.getTimeSegmentVsWorkerMap().isEmpty(). Please let me know if this looks good.

[cryptoe]

Thanks for the changes @gianm