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

Not an issue, but a basic question #302

Closed
wind57 opened this issue Oct 5, 2021 · 7 comments
Closed

Not an issue, but a basic question #302

wind57 opened this issue Oct 5, 2021 · 7 comments
Labels

Comments

@wind57
Copy link

wind57 commented Oct 5, 2021

Suppose I have an action, that if fails, triggers a fallback. As simple as it can get:

public class FailSafeMissUnderstanding {


	public static void main(String[] args) {

		RetryPolicy<String> retryPolicy = new RetryPolicy<String>()
				.withMaxAttempts(1)
				.onSuccess(x -> System.out.println("main call provided the result"))
				.onFailure(x -> System.out.println("main call failed"));

		Failsafe.with(firstFallback(), retryPolicy).get(() -> { return "OK"; });
	}

	private static Fallback<String> firstFallback() {
		return Fallback.of(new FailSafeMissUnderstanding()::firstFallbackCall)
				.handleIf(Objects::nonNull)
				.onSuccess(result -> System.out.println("first fallback provided the result "  + result));
	}

	private String firstFallbackCall() {
		System.out.println("executing first fallback");
		return "just fine first callback";
	}

}

This works just fine, but somehow I miss-understand the logs. Running this, I get :

main call provided the result
first fallback provided the result ExecutionCompletedEvent[result=OK, failure=null]

Why is onSuccess from the Fallback triggered? May be I miss the obvious here... what I would like to achieve is to log which of the methods got my correct result. Be that the "main call" or the "fallback".

Thank you.

@Tembrel
Copy link
Contributor

Tembrel commented Oct 5, 2021

You told the fallback to handleIf(Objects::nonNull), and the result of the main call is non-null. So the retry policy succeeds, and the fallback handles this non-null result by calling firstFallbackCall.

Update

This is wrong, see below.

@wind57
Copy link
Author

wind57 commented Oct 5, 2021

I am not using handleResultIf, but handleIf, which takes a Throwable, not the result of the "main call". Your explanation would have been correct for the handleResultIf case, right?

@Tembrel
Copy link
Contributor

Tembrel commented Oct 5, 2021

You're right, and in fact handleIf isn't being called at all, because there is no failure to test. The onSuccess handler of the fallback is called because the overall handling to that point was a success.

I think you want something like Fallback.onFailedAttempt, which was recently (#298) made distinct from Fallback.onFailure.

@wind57
Copy link
Author

wind57 commented Oct 5, 2021

that's not what I want though (or I miss-understand your point). I want to be able to log which part of the code provided the actual result, the supplier or the fallback. In my case the supplier did not fail, so I expect .onSuccess(x -> System.out.println("main call provided the result")) to be triggered, which in fact happens.

Since the fallback is not really doing anything, why does it trigger onSuccess too? Is there something I miss in my set-up? I hope this makes sense.

The real use case that I have is much more involved, but it has the same idea - we have many fallbacks and we would like to be able to log - which of those actually provided the correct result.

@wind57
Copy link
Author

wind57 commented Oct 5, 2021

What I thought (though I really don't like it), is may be be able to do something like this:

	public static void main(String[] args) {

		RetryPolicy<String> retryPolicy = new RetryPolicy<String>()
				.withMaxAttempts(1)
				//.onSuccess(x -> System.out.println("main call provided the result"))
				.onFailure(x -> System.out.println("main call failed"));

		Failsafe.with(firstFallback(), retryPolicy).get(() -> { return "OK"; });
	}

	private static Fallback<String> firstFallback() {
		return Fallback.of(new FailSafeMissUnderstanding()::firstFallbackCall)
				.handleIf(anyT -> {
					System.out.println("throwable is null ? " + (anyT == null));
					return anyT != null;
				})
				.handleResultIf(x -> {
					if(x != null) {
						System.out.println("main call did the work");
					}

					return false;
				});
				//.onSuccess(result -> System.out.println("first fallback provided the result "  + result));
	}

Notice the:

.handleResultIf(x -> {
	if(x != null) {
		System.out.println("main call did the work");
	}

	return false;
});

and also the removal of onSuccess. This looks dirty to me since I am piggy backing in the current policy on what might have happened in the previous one and as such, order matters (if I add one more policy, I need to change all the log messages) but let's suppose we are OK with that. The problem arises that now I will get two log entries:

main call did the work
main call did the work

@Tembrel
Copy link
Contributor

Tembrel commented Oct 5, 2021

If you look through other Failsafe issues you'll see a lot of confusion and debate around the meaning of success and failure with respect to fallbacks. The methods onSuccess and onFailure are defined in subtype PolicyListeners and are defined generically in terms of whether the policy itself succeeds or fails in handling an execution.

In the case of a fallback, where the whole idea is to get a successful result even if the execution enclosed by the fallback fails, you would expect onSuccess to be called every time. The only time onFailure would be called would be if the supplier for the fallback itself fails in some way.

The reason I suggested onFailedAttempt above is because it's the only listener method on Fallback that distinguishes between an "incoming" success vs. failure; it is called if the preceding execution was a failure. (The method with the same name on RetryPolicy is subtly different.) You'd need Failsafe 2.4.4 or later to use it, though, and it might be at least as awkward to use as the workaround you wrote about.

The problem with using listeners (or instrumenting methods like handleResultIf, as you described) in complex policy chains is that there's no way in general to guarantee that you won't get repeated calls to listeners. You could set up a tracing system that records calls to say that a particular source's result is being used, yielding the last source reported.

Or you could build the source into the result value:

public class SourcedResult<R, S> {
    private final R result;
    private final S source;

    public static <R, S> SourcedResult<R, S> of(R result, S source) {
        return new SourcedResult<R, S>(result, source);
    }

    private SourcedResult(R result, S source) {
        this.result = result;
        this.source = source;
    }

    public R getResult() { return result; }
    public S getSource() { return source; }

    @Override public String toString() {
        return String.format("%s(%s)", result, source);
    }
}

I adapted your example in the issue description to use these sourced results:

    enum Source { MAIN, FALLBACK }

    static void revisedExample() {

        RetryPolicy<SourcedResult<String, Source>> retryPolicy = new RetryPolicy<SourcedResult<String, Source>>()
            .withMaxAttempts(1)
            .onSuccess(x -> System.out.println("main call provided the result"))
            .onFailure(x -> System.out.println("main call failed"));

        Fallback<SourcedResult<String, Source>> fallback = Fallback.of(() -> {
            System.out.println("executing first fallback");
            return SourcedResult.of("just fine first callback", Source.FALLBACK);
        })
            .handleIf(Objects::nonNull)
            .onSuccess(result -> System.out.println("fallback provided a result "  + result));

        Failsafe.with(fallback, retryPolicy).get(() -> { return SourcedResult.of("OK", Source.MAIN); });
    }

and the output clarifies that even though the fallback's success listener is triggered, the result being reported is from the main call:

main call provided the result
fallback provided a result ExecutionCompletedEvent[result=OK(MAIN), failure=null]

@wind57
Copy link
Author

wind57 commented Oct 5, 2021

funny that we ended up writing close to the same exact pattern, we call Source -> CallerDestination (now that I think about it, its not a great name either).

I am, btw, in no shape or form complaining about the choices made; just wanted to make it clear for myself.

Thank you for you elaborate answer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants