Skip to content

Commit 83b49d4

Browse files
bobbai00claude
andcommitted
refactor(execution-service): tidy SyncExecutionResource pure helpers
Shrink the sampling helpers' return to the three consumed values, extract the duplicated back-window loop into collectBackWindow, and drop the constructor-echo tests in favor of the behavioral suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 35b5346 commit 83b49d4

2 files changed

Lines changed: 86 additions & 193 deletions

File tree

amber/src/main/scala/org/apache/texera/web/resource/SyncExecutionResource.scala

Lines changed: 71 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ class SyncExecutionResource extends LazyLogging {
436436
val stats = operatorStats.get(opId)
437437
val state = stats.map(s => stateToString(s.operatorState)).getOrElse("Unknown")
438438

439-
val (resultMode, result, tuplesCount, _, _) =
439+
val (resultMode, result, tuplesCount) =
440440
collectOperatorResult(
441441
executionId,
442442
opId,
@@ -561,7 +561,7 @@ class SyncExecutionResource extends LazyLogging {
561561
opId: String,
562562
maxOperatorResultCharLimit: Int,
563563
maxOperatorResultCellCharLimit: Int
564-
): (String, Option[List[SampleRow]], Option[Int], Option[Int], Option[Boolean]) = {
564+
): (String, Option[List[SampleRow]], Option[Int]) = {
565565
try {
566566
val storageUriOption = WorkflowExecutionsResource.getResultUriByLogicalPortId(
567567
executionId,
@@ -584,12 +584,12 @@ class SyncExecutionResource extends LazyLogging {
584584
)
585585

586586
case None =>
587-
("table", None, None, None, None)
587+
("table", None, None)
588588
}
589589
} catch {
590590
case e: Exception =>
591591
logger.warn(s"Error collecting result for operator $opId: ${e.getMessage}", e)
592-
("table", None, None, None, None)
592+
("table", None, None)
593593
}
594594
}
595595

@@ -604,11 +604,11 @@ class SyncExecutionResource extends LazyLogging {
604604
totalCount: Int,
605605
maxOperatorResultCharLimit: Int,
606606
maxOperatorResultCellCharLimit: Int
607-
): (String, Option[List[SampleRow]], Option[Int], Option[Int], Option[Boolean]) = {
607+
): (String, Option[List[SampleRow]], Option[Int]) = {
608608
val mapper = new ObjectMapper()
609609

610610
if (totalCount == 0 || !tupleIterator.hasNext) {
611-
return ("table", Some(List.empty[SampleRow]), Some(0), Some(0), Some(false))
611+
return ("table", Some(List.empty[SampleRow]), Some(0))
612612
}
613613

614614
// A single tuple with html-content / json-content is a visualization payload —
@@ -618,7 +618,7 @@ class SyncExecutionResource extends LazyLogging {
618618
val jsonResults =
619619
ExecutionResultService.convertTuplesToJson(List(firstTuple), isVisualization = true)
620620
val rows = jsonResults.zipWithIndex.map { case (json, idx) => SampleRow(idx, json) }
621-
return ("visualization", Some(rows), Some(totalCount), Some(1), Some(false))
621+
return ("visualization", Some(rows), Some(totalCount))
622622
}
623623

624624
// rowIndex preserves the original position so the client can show "row N"
@@ -629,13 +629,7 @@ class SyncExecutionResource extends LazyLogging {
629629
val firstSize = estimateTupleSize(truncatedFirst, mapper)
630630

631631
if (firstSize >= maxOperatorResultCharLimit) {
632-
return (
633-
"table",
634-
Some(List(SampleRow(rowIndex, truncatedFirst))),
635-
Some(totalCount),
636-
Some(1),
637-
Some(true)
638-
)
632+
return ("table", Some(List(SampleRow(rowIndex, truncatedFirst))), Some(totalCount))
639633
}
640634

641635
val halfLimit = maxOperatorResultCharLimit / 2
@@ -656,65 +650,76 @@ class SyncExecutionResource extends LazyLogging {
656650
frontRows += row
657651
frontSize += tupleSize
658652
} else {
659-
// Front is full — switch to a sliding window for the back half.
660-
val backBuffer = mutable.ArrayBuffer[(SampleRow, Int)]()
661-
backBuffer += ((row, tupleSize))
662-
var backSize = tupleSize
663-
664-
while (tupleIterator.hasNext) {
665-
val t = tupleIterator.next()
666-
rowIndex += 1
667-
val jt = ExecutionResultService.convertTuplesToJson(List(t)).head
668-
val tt = truncateSingleTuple(jt, maxOperatorResultCellCharLimit)
669-
val ts = estimateTupleSize(tt, mapper)
670-
671-
backBuffer += ((SampleRow(rowIndex, tt), ts))
672-
backSize += ts
673-
674-
while (backSize > halfLimit - truncationNoticeSize && backBuffer.size > 1) {
675-
val (_, removedSize) = backBuffer.remove(0)
676-
backSize -= removedSize
677-
}
678-
}
679-
680-
val allRows = frontRows.toList ++ backBuffer.map(_._1).toList
681-
val skippedRows = totalCount - allRows.size
682-
return (
683-
"table",
684-
Some(allRows),
685-
Some(totalCount),
686-
Some(allRows.size),
687-
Some(skippedRows > 0)
653+
// Front is full — switch to a sliding window for the back half, seeded with the
654+
// row that overflowed the front.
655+
val backRows = collectBackWindow(
656+
tupleIterator,
657+
rowIndex,
658+
Some((row, tupleSize)),
659+
halfLimit - truncationNoticeSize,
660+
maxOperatorResultCellCharLimit,
661+
mapper
688662
)
663+
return ("table", Some(frontRows.toList ++ backRows), Some(totalCount))
689664
}
690665
}
691666

692667
if (tupleIterator.hasNext) {
693-
val backBuffer = mutable.ArrayBuffer[(SampleRow, Int)]()
694-
var backSize = 0
695-
696-
while (tupleIterator.hasNext) {
697-
val t = tupleIterator.next()
698-
rowIndex += 1
699-
val jt = ExecutionResultService.convertTuplesToJson(List(t)).head
700-
val tt = truncateSingleTuple(jt, maxOperatorResultCellCharLimit)
701-
val ts = estimateTupleSize(tt, mapper)
702-
703-
backBuffer += ((SampleRow(rowIndex, tt), ts))
704-
backSize += ts
705-
706-
while (backSize > halfLimit - truncationNoticeSize && backBuffer.size > 1) {
707-
val (_, removedSize) = backBuffer.remove(0)
708-
backSize -= removedSize
709-
}
710-
}
711-
712-
val allRows = frontRows.toList ++ backBuffer.map(_._1).toList
713-
val skippedRows = totalCount - allRows.size
714-
("table", Some(allRows), Some(totalCount), Some(allRows.size), Some(skippedRows > 0))
668+
val backRows = collectBackWindow(
669+
tupleIterator,
670+
rowIndex,
671+
None,
672+
halfLimit - truncationNoticeSize,
673+
maxOperatorResultCellCharLimit,
674+
mapper
675+
)
676+
("table", Some(frontRows.toList ++ backRows), Some(totalCount))
715677
} else {
716-
("table", Some(frontRows.toList), Some(totalCount), Some(frontRows.size), Some(false))
678+
("table", Some(frontRows.toList), Some(totalCount))
679+
}
680+
}
681+
682+
/**
683+
* Consume the remaining tuples into a sliding back-window that keeps the most recent rows
684+
* within `backSizeLimit` estimated characters, always retaining at least one row. `seed`
685+
* is the already-processed row (and its size) that overflowed the front half, if any.
686+
* Shared by the "front already full" and trailing back-window branches of
687+
* `sampleAndTruncateTuples`.
688+
*/
689+
private def collectBackWindow(
690+
tupleIterator: Iterator[Tuple],
691+
startRowIndex: Int,
692+
seed: Option[(SampleRow, Int)],
693+
backSizeLimit: Int,
694+
maxOperatorResultCellCharLimit: Int,
695+
mapper: ObjectMapper
696+
): List[SampleRow] = {
697+
val backBuffer = mutable.ArrayBuffer[(SampleRow, Int)]()
698+
var backSize = 0
699+
seed.foreach {
700+
case (row, size) =>
701+
backBuffer += ((row, size))
702+
backSize += size
703+
}
704+
705+
var rowIndex = startRowIndex
706+
while (tupleIterator.hasNext) {
707+
val t = tupleIterator.next()
708+
rowIndex += 1
709+
val jt = ExecutionResultService.convertTuplesToJson(List(t)).head
710+
val tt = truncateSingleTuple(jt, maxOperatorResultCellCharLimit)
711+
val ts = estimateTupleSize(tt, mapper)
712+
713+
backBuffer += ((SampleRow(rowIndex, tt), ts))
714+
backSize += ts
715+
716+
while (backSize > backSizeLimit && backBuffer.size > 1) {
717+
val (_, removedSize) = backBuffer.remove(0)
718+
backSize -= removedSize
719+
}
717720
}
721+
722+
backBuffer.map(_._1).toList
718723
}
719724

720725
private def truncateSingleTuple(

0 commit comments

Comments
 (0)