Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(trivial) removes some hardcoded fate values #5243

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

kevinrr888
Copy link
Member

Removes some hardcoded values used by Fate and Fate tests.

Removes some hardcoded values used by Fate and Fate tests.
@@ -238,7 +238,7 @@ protected void testTransactionStatus(FateStore<TestEnv> store, ServerContext sct
try {

// Wait for the transaction runner to be scheduled.
Thread.sleep(3000);
Thread.sleep(fate.getInitialDelay().toMillis() * 2);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

greater than the delay to avoid potential timing issues

@kevinrr888 kevinrr888 self-assigned this Jan 10, 2025
@@ -476,10 +477,18 @@ public Fate(T environment, FateStore<T> store, boolean runDeadResCleaner,
this.workFinder.start();
}

public Duration getInitialDelay() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the plan to allow overriding these methods? If not maybe they could be public and statically defined constants in Fate? Even if we want to keep the methods to allow overriding they could delegate to the statically defined constants (if you keep the methods they could be private). This way we are not creating new objects each time the method is called.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions. We shouldn't allow overriding for initial delay, so I removed that. getDeadResCleanupDelay is already overridden in FastFate and I also plan to override getPoolWatcherDelay in FastFate as well in an upcoming PR, so left those methods but no longer create a new Duration each time.

@kevinrr888 kevinrr888 merged commit 862b6e5 into apache:main Jan 13, 2025
8 checks passed
@kevinrr888 kevinrr888 deleted the 4.0-feature-fate-hardcoded-waits branch January 13, 2025 14:41
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants