Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ jobs:
"Auth/jacoco" \
"Config/jacoco" \
"Resource/jacoco" \
"Util/jacoco" \
"PyBuilder/jacoco" \
"WorkflowCore/jacoco" \
"WorkflowOperator/jacoco" \
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ engine, an Angular UI, and the agent service. JVM modules wired in
| --- | --- | --- |
| Workflow execution engine (Amber) | `amber/` | [amber/README.md](amber/README.md) |
| Backend services | `config-service/`, `access-control-service/`, `file-service/`, `computing-unit-managing-service/`, `workflow-compiling-service/`, `notebook-migration-service/` | `build.sbt` |
| Shared Scala libs | `common/` (`auth`, `config`, `dao`, `workflow-core`, `workflow-operator`, `pybuilder`) | `build.sbt` |
| Shared Scala libs | `common/` (`auth`, `config`, `dao`, `util`, `workflow-core`, `workflow-operator`, `pybuilder`) | `build.sbt` |
| Frontend (Angular) | `frontend/` | [frontend/README.md](frontend/README.md) |
| Agent service (Bun/TS, LLM agents) | `agent-service/` | `agent-service/package.json` |
| Pyright language service | `pyright-language-service/` | [pyright-language-service/README.md](pyright-language-service/README.md) |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ object Utils extends LazyLogging {
* for callers on an actor or coordinator thread, where a `Thread.sleep` would also stall
* unrelated work queued on that thread.
*
* This is the non-blocking half of the pair: for blocking work, use
* `org.apache.texera.common.util.RetryUtil.withBackoff` in `common/util`, which takes the same
* attempts-and-doubling knobs. It lives in a separate module because this variant needs
* `com.twitter:util-core`, which only `amber` declares.
Comment thread
aglinxinyuan marked this conversation as resolved.
*
* A synchronous throw while evaluating `fn` counts as a failed attempt, same as a failed
* `Future`. Fatal errors are never retried in either shape: they propagate immediately.
*
Expand All @@ -88,8 +93,8 @@ object Utils extends LazyLogging {
)(fn: => Future[T]): Future[T] = {
def attempt(attemptNumber: Int, backoffTimeInMS: Long): Future[T] =
Future(fn).flatten.rescue {
// `NonFatal` so that a fatal handed back as a failed `Future` is not retried either, which
// also matches how the blocking backoff loops elsewhere in the repo catch `Exception`.
// `NonFatal` so that a fatal handed back as a failed `Future` is not retried either, matching
// the blocking sibling `RetryUtil.withBackoff`, which uses the same `NonFatal` predicate.
case NonFatal(e) if attemptNumber < attempts =>
onRetry(e, attemptNumber, backoffTimeInMS)
Future
Expand Down
8 changes: 6 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@
// keep the org.apache.log4j API available at runtime.
ThisBuild / excludeDependencies += ExclusionRule("log4j", "log4j")

// Dependency-free helpers (retry/backoff, ...) that any module may depend on. Keep it that way:
// anything added here reaches the classpath of every service that depends on it.
lazy val Util = (project in file("common/util")).settings(commonModuleSettings)
Comment thread
aglinxinyuan marked this conversation as resolved.
lazy val DAO = (project in file("common/dao")).settings(commonModuleSettings)
lazy val Config = (project in file("common/config")).settings(commonModuleSettings)
lazy val Resource = (project in file("common/resource")).settings(commonModuleSettings)
Expand Down Expand Up @@ -156,7 +159,7 @@

lazy val WorkflowCore = (project in file("common/workflow-core"))
.settings(commonModuleSettings)
.dependsOn(DAO, Config, PyBuilder)
.dependsOn(DAO, Config, PyBuilder, Util)
.configs(Test)
.dependsOn(DAO % "test->test") // test scope dependency
lazy val ComputingUnitManagingService = (project in file("computing-unit-managing-service"))
Expand Down Expand Up @@ -202,7 +205,7 @@
)
lazy val FileService = (project in file("file-service"))
.settings(commonModuleSettings)
.dependsOn(WorkflowCore, Auth, Config, Resource)
.dependsOn(WorkflowCore, Auth, Config, Resource, Util)
.configs(Test)
.dependsOn(DAO % "test->test") // test scope dependency
.settings(
Expand All @@ -217,10 +220,10 @@
// so every suite gets its own forked JVM; sbt runs forked groups one at a
// time by default (Tags.ForkedTestGroup limit), keeping the stacks serial.
Test / fork := true,
Test / forkOptions := (Test / forkOptions).value

Check warning on line 223 in build.sbt

View workflow job for this annotation

GitHub Actions / Bench

The evaluation of `/` inside an anonymous function is prohibited.
.withWorkingDirectory((ThisBuild / baseDirectory).value),
Test / testGrouping := (Test / definedTests).value.map { suite =>
Tests.Group(suite.name, Seq(suite), Tests.SubProcess((Test / forkOptions).value))

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / Bench

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / amber-integration (macos-latest, 17)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / amber (ubuntu-latest, 17)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / amber-integration (ubuntu-latest, 17)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform (workflow-compiling-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform (computing-unit-managing-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform (access-control-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform-integration (computing-unit-managing-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform-integration (config-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform-integration (notebook-migration-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform-integration (workflow-compiling-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform-integration (access-control-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform (config-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform-integration (file-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform (notebook-migration-service)

The evaluation of `/` inside an anonymous function is prohibited.

Check warning on line 226 in build.sbt

View workflow job for this annotation

GitHub Actions / build / platform (file-service)

The evaluation of `/` inside an anonymous function is prohibited.
}
)

Expand Down Expand Up @@ -278,6 +281,7 @@
Auth,
Config,
Resource,
Util,
DAO,
PyBuilder,
WorkflowCore,
Expand Down
56 changes: 56 additions & 0 deletions common/util/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

import scala.collection.Seq

name := "util"

enablePlugins(JavaAppPackaging)

// Enable semanticdb for Scalafix
ThisBuild / semanticdbEnabled := true
ThisBuild / semanticdbVersion := scalafixSemanticdb.revision

// Manage dependency conflicts by always using the latest revision
ThisBuild / conflictManager := ConflictManager.latestRevision

// Restrict parallel execution of tests to avoid conflicts
Global / concurrentRestrictions += Tags.limit(Tags.Test, 1)

/////////////////////////////////////////////////////////////////////////////
// Compiler Options
/////////////////////////////////////////////////////////////////////////////

// Scala compiler options
Compile / scalacOptions ++= Seq(
"-Xelide-below", "WARNING", // Turn on optimizations with "WARNING" as the threshold
"-feature", // Check feature warnings
"-deprecation", // Check deprecation warnings
"-Ywarn-unused:imports" // Check for unused imports
)

/////////////////////////////////////////////////////////////////////////////
// Dependencies
/////////////////////////////////////////////////////////////////////////////

// This module is deliberately dependency-free apart from the test framework:
// any module may depend on it, so anything added here reaches the classpath of
// every service that does. Callers pass their own logger in through the hooks
// instead of the module pulling in a logging library.
libraryDependencies ++= Seq(
"org.scalatest" %% "scalatest" % "3.2.15" % Test // ScalaTest (for unit tests)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.texera.common.util

import scala.annotation.tailrec
import scala.util.control.NonFatal

/**
* Retry with exponential backoff, for blocking work.
*
* This module has no dependencies of its own, so any module may depend on it rather than writing
* another retry loop by hand. If the work returns a `Future` rather than blocking, use the
* non-blocking sibling
* `org.apache.texera.amber.engine.common.Utils.retry` in `amber` instead: it takes the same
* attempts-and-doubling-backoff knobs but waits on a `Timer`, which matters on an actor or
* coordinator thread where `Thread.sleep` would stall unrelated work queued behind it.
*/
object RetryUtil {

/**
* One failed attempt that is about to be retried. Carries everything a caller needs to log the
* retry itself; `message` is the standard wording, so retries read the same everywhere.
*/
final case class RetryAttempt(
description: String,
attempt: Int,
maxAttempts: Int,
delayMillis: Long,
cause: Throwable
) {
def message: String =
s"Failed to $description (attempt $attempt/$maxAttempts): ${cause.getMessage}. " +
s"Retrying in ${delayMillis}ms..."
}

/**
* Runs `operation`, retrying on failure with exponential backoff (the delay doubles after each
* failed attempt) until it succeeds or `maxAttempts` is reached. The final failure is wrapped
* with `description` and the last exception as its cause.
*
* Only `NonFatal` failures are treated as transient, which is the same predicate the
* non-blocking sibling uses. Note that `NonFatal` admits non-fatal `Error`s -- `AssertionError`,
* `java.io.IOError`, `ServiceConfigurationError` -- so those are retried rather than propagated
* straight away. An `InterruptedException` -- raised by the operation or by the wait between
* attempts -- fails fast with the interrupt status restored, so a caller shutting the thread
* down is never made to sit through the remaining backoff.
*
* @param description verb phrase naming the work, e.g. "connect to lake fs server". It is
* interpolated into every message: "Failed to $description after ...".
* @param maxAttempts total attempts; 1 means no retry at all.
* @param initialDelayMillis wait before the first retry; doubled after each failed attempt.
* @param onRetry invoked before each wait. Log `RetryAttempt.message` through the
* caller's own logger, so retries are attributed to the caller rather
* than to this util.
* @param sleep how to wait; injectable so tests exercise the backoff without waiting.
* @param operation the work to run, re-evaluated on each attempt.
* @tparam T whatever `operation` returns.
* @return `operation`'s value from the first attempt that succeeds.
*/
def withBackoff[T](
description: String,
maxAttempts: Int,
initialDelayMillis: Long,
onRetry: RetryAttempt => Unit,
sleep: Long => Unit = Thread.sleep
)(operation: => T): T = {
// Restore the interrupt status and fail fast rather than retrying, whether the interrupt
// arrives while running `operation` or while waiting between attempts.
def failInterrupted(cause: InterruptedException): Nothing = {
Thread.currentThread().interrupt()
throw new RuntimeException(s"Interrupted while waiting to $description", cause)
}

@tailrec
def attemptFrom(attempt: Int, delayMillis: Long): T = {
val outcome: Either[Throwable, T] =
try Right(operation)
catch {
case ie: InterruptedException => failInterrupted(ie)
case NonFatal(cause) => Left(cause)
Comment thread
aglinxinyuan marked this conversation as resolved.
}

outcome match {
case Right(value) => value
case Left(cause) =>
if (attempt >= maxAttempts) {
throw new RuntimeException(
s"Failed to $description after $maxAttempts attempts: ${cause.getMessage}",
cause
)
}
onRetry(RetryAttempt(description, attempt, maxAttempts, delayMillis, cause))
try sleep(delayMillis)
catch { case ie: InterruptedException => failInterrupted(ie) }
attemptFrom(attempt + 1, delayMillis * 2)
}
}

attemptFrom(attempt = 1, delayMillis = initialDelayMillis)
}
}
Loading
Loading