State diagrams

[cameel]

This is the state diagram from #139 blueprint and another, simpler one showing the same thing. It is supposed to have the same states and transitions but without detailing conditions in which the transition happens.

Before reviewing please read #431 (comment). Same rules apply here.

[rwrzesien]

@cameel

1. Why in Report case there is CMT used twice ?
2. In code in Report FRCTR and VRCT becomes available after TTC.deadline + CMT, not after TTC.deadline.
3. In code in ResultTranfer we FGTRF becomes available after RCT.timestamp + FAT + CMT. This is not visible on this schema.
4. In code in Acceptance, time window is based on CTD.deadline instead of TTC.deadline.
5. Why in Acceptance case there is CMT used twice ?
6. Maybe it would be possible somehow to show that in Payment SubtaskResultAccepted.payment_ts is the oldest one from collection ?
7. Why in AdditionalVerification case there is SVT used ?

[kbeker]

Generally OK, but:
Some legend to this diagram would be nice. If someone see for the first time this diagram it won't be obvious for reader why some of these states are yellow or blue. Also color of messages: why some of these messages are orange or blue? I don't know if it should be like this but some of messages names are on the top of few lines e.g SubtaskResultsVerify before VERIFICATION FILE TRANSFER state, also `all files uplodaded below VFT state.

[rwrzesien]

Please clarify the questions.

[dybi]

1. in ADDITIONAL VERIFICATION, either timeout should be t > rejection + AVCT + SVT or verification successful should be t < rejection + AVCT + SVT; right now both cases are NOT mutally exclusive
2. the same applies for VERIFICATION FILE TRANSFER
3. shouldn't be there some additional margin for ADDITIONAL VERIFICATION? Now both have exacly the same timeout
4. OK, I see that in almost all cases timeout is not mutally exclusive with the second path (becasue of =). Exception would be ADDITIONAL VERIFICATION: int code there is

    if not current_time <= subtask_results_rejected.timestamp + settings.ADDITIONAL_VERIFICATION_CALL_TIME:
        return message.concents.ServiceRefused(
            reason=message.concents.ServiceRefused.REASON.InvalidRequest,
        )

5. In code timeout happens with the = sign, so all conditions should have t < SOME + VARIABLES + AND + TIMESTAMPS
6. FORCE GET TASK RESULTS deadline is:

force_get_task_result_deadline = (
        client_message.report_computed_task.task_to_compute.compute_task_def['deadline'] +
        2 * settings.CONCENT_MESSAGING_TIME +
        calculate_maximum_download_time(client_message.report_computed_task.size)
    )

(NOT report + MDT + MAT + 2MMTT),
also in the code there is no check that report + MDT < t
7) In FORCE GET TASK RESULTS, deadline (in the code) for file uploads IS: report.timestamp + FAT +CMT and NOT report + 2MDT + 3MAT + 4MMTT + CMT

More general comments:

• it would be nice to have a legend, stating what colors mean what for messages and for states
• I assume that green arrow is "positive" scenario and red is "failed". Black are, let's say, entries. If my understanding is correct, shouldn't the arrow between VERIFICATION FILE TRANSFER and ADDITIONAL VERIFICATION be green?

[cameel]

@rwrzesien It sounds like your remarks are for the timing diagram, not state diagram. I will respond in #431.

@kbeker

Some legend to this diagram would be nice. If someone see for the first time this diagram it won't be obvious for reader why some of these states are yellow or blue.

Done.

Also color of messages: why some of these messages are orange or blue?

Orange ones are from the provider, blue ones are from the requestor. It's now in the description.

I don't know if it should be like this but some of messages names are on the top of few lines e.g SubtaskResultsVerify before VERIFICATION FILE TRANSFER state, also `all files uplodaded below VFT state.

They're over the edges coming out of different states. The same message can be accepted when the subtask is in different states.

[cameel]

1. in ADDITIONAL VERIFICATION, either timeout should be t > rejection + AVCT + SVT or verification successful should be t < rejection + AVCT + SVT; right now both cases are NOT mutally exclusive

2. the same applies for VERIFICATION FILE TRANSFER

4. OK, I see that in almost all cases timeout is not mutally exclusive with the second path (becasue of =).

Fixed.

The idea behind this when I first created those diagrams was that each transition was supposed to have a specific trigger. I wanted it to show that specifically t = rejection + AVCT + SVT triggers the transition just like in other cases receiving a message triggers a transition.

But I see that this is confusing so I changed the conditions. Now they're mutally exclusive.

5. In code timeout happens with the = sign, so all conditions should have t < SOME + VARIABLES + AND + TIMESTAMPS

I changed it all to <=. I think that it makes slightly more sense if the right end of the interval is the closed one. E.g. when you have a deadline until, say, 2018-05-05, normally the day of 2018-05-05 is still before the deadline.

3. shouldn't be there some additional margin for ADDITIONAL VERIFICATION? Now both have exacly the same timeout

I'm not sure what you mean.
For VERIFICATION FILE TRANSFER the timeout is rejection + AVCT.
For ADDITIONAL VERIFICATION the timeout is rejection + AVCT + SVT.
So they're not the same. The margin is SVT.

6. FORCE GET TASK RESULTS deadline is:
   (...)
   (NOT report + MDT + MAT + 2MMTT),

Fixed. Looks like I updated this on the timing diagram but I forgot to fix the state diagram as well.

also in the code there is no check that report + MDT < t

That check should be there. If it's not, it is a bug, please report it.

That being said, the correct condition is report < t now and the impact of not having that check is not big because the requestor won't be able to start this use case until he receives RCT from the provider.

7. In FORCE GET TASK RESULTS, deadline (in the code) for file uploads IS: report.timestamp + FAT +CMT and NOT report + 2MDT + 3MAT + 4MMTT + CMT

If it's based on FAT in the code then it's probably still the old implementation based on #12. It was supposed to be fixed in #166.

Also, the condition should be based on TaskToCompute.deadline, not ReportComputedTask.timestamp. See #165. Please report this as a bug.

• it would be nice to have a legend, stating what colors mean what for messages and for states

Done.

• I assume that green arrow is "positive" scenario and red is "failed". Black are, let's say, entries. If my understanding is correct, shouldn't the arrow between VERIFICATION FILE TRANSFER and ADDITIONAL VERIFICATION be green?

Right. Fixed.

The green ones are outcomes that are positive for the one who started the use case. Red ones are negative. Black ones are just neutral transitions.

[cameel]

I have uploaded the updated diagram to the #139 blueprint too. Here's a short summary of all changes made so far: #139 (comment).

[cameel]

@dybi

3. shouldn't be there some additional margin for ADDITIONAL VERIFICATION? Now both have exacly the same timeout

I'm not sure what you mean.
For VERIFICATION FILE TRANSFER the timeout is rejection + AVCT.
For ADDITIONAL VERIFICATION the timeout is rejection + AVCT + SVT.
So they're not the same. The margin is SVT.

Now that I think about it, you probably mean the timeout for file upload and for VERIFICATION FILE TRANSFER?

AVCT interval is supposed to be long enough to allow the provider to submit SubtaskResultsVerify and to upload all the necessary files. Łukasz's specification does not say how much time should be allotted to each of these things. We could split this interval into parts but how exactly? I don't think there's any harm in leaving it as is - if the provider decides to wait for the last possible moment to submit SubtaskResultsVerify and then is not able to upload everything on time then that's his fault.

[kbeker]

@cameel
In Legend ForceReportComputedTask is hooding the beginning of the description for message colors.

I don't know if it should be like this but some of messages names are on the top of few lines e.g SubtaskResultsVerify before VERIFICATION FILE TRANSFER state, also `all files uplodaded below VFT state.

They're over the edges coming out of different states. The same message can be accepted when the subtask is in different states.

This is not what I meant. Between VERIFICATION FILE TRANSFER and ADDIDTIONAL VERIFICATION state is sentence all files uploaded which is hooding arrow. I think that it should be moved a little bit left - same situation which SubtaskResultsVerify between REJECTED and VERIFICATION FILE TRANSFER states

[dybi]

That check should be there. If it's not, it is a bug, please report it.

That being said, the correct condition is report < t now and the impact of not having that check is not big because the requestor won't be able to start this use case until he receives RCT from the provider.

New: #461

Also, the condition should be based on TaskToCompute.deadline, not ReportComputedTask.timestamp. See #165. Please report this as a bug.

Fixed: #452

[dybi]

@cameel, 2 more things:

1. SubtaskResultsVerify between REJECTED and VERIFICATION FILE TRANSFER has different time restrictions (namely, it lacks rejection < t part)
2. rejection < t part doesn't seem to be implemented in handle_send_subtask_results_verify

EDIT:
I have created an issue for the latter: #462

[dybi]

@cameel ,

The green ones are outcomes that are positive for the one who started the use case. Red ones are negative. Black ones are just neutral transitions.

It the legend there is transition to a passive states indicating rejection/acceptance of provider's claim. I think it should be client's claim as in FORCING RESULT TRANSFER scenario it is requstor whose claims are being processed

[cameel]

1. SubtaskResultsVerify between REJECTED and VERIFICATION FILE TRANSFER has different time restrictions (namely, it lacks rejection < t part)

That's on purpose. We're already in the REJECTED state so there's no need to check the timestamp to make sure that there was a rejection.

Actually, I think I'm going to remove the lower limits from the other cases of this message to make it consistent with #532. But I'm not adding this or changes from #532 to the diagram yet because I want the diagram to match the current code.

The green ones are outcomes that are positive for the one who started the use case. Red ones are negative. Black ones are just neutral transitions.

It the legend there is transition to a passive states indicating rejection/acceptance of provider's claim. I think it should be client's claim as in FORCING RESULT TRANSFER scenario it is requstor whose claims are being processed

Sorry, what I said above was incorrect. It's positive for the provider, not for the one who started the case.

The diagram is correct. Green is alwys positive for the provider, not for the one who started. In FORCING RESULT TRANSFER requestor's claim is that the provider does not have the result. Provider's claim is the opposite and he can prove it by uploading the result.

Do you think I should reword it?

[cameel]

@kbeker

In Legend ForceReportComputedTask is hooding the beginning of the description for message colors.

I don't know if it should be like this but some of messages names are on the top of few lines e.g SubtaskResultsVerify before VERIFICATION FILE TRANSFER state, also `all files uplodaded below VFT state.

They're over the edges coming out of different states. The same message can be accepted when the subtask is in different states.

This is not what I meant. Between VERIFICATION FILE TRANSFER and ADDIDTIONAL VERIFICATION state is sentence all files uploaded which is hooding arrow. I think that it should be moved a little bit left - same situation which SubtaskResultsVerify between REJECTED and VERIFICATION FILE TRANSFER states

I don't see it. The one between VERIFICATION FILE TRANSFER and ADDITIONAL VERIFICATION slightly touches the arrow but that's it. Is it visible on the PNG version in #139 too?

It might be a rendering difference in SVG. For example due to some fonts missing and others being subtituted. I may be able to fix it by using different fonts but I first have to see it.

[cameel]

Diagrams updated. Summary of changes:

• There was a typo: ForgeGetTaskResult instead of ForceGetTaskResult
• The condition for all files uploaded in FORCING RESULT TRANSFER was wrong. It was t > deadline + 2MDT + 3CMT but should have been t <= deadline + 2MDT + 3CMT.

[cameel]

1. SubtaskResultsVerify between REJECTED and VERIFICATION FILE TRANSFER has different time restrictions (namely, it lacks rejection < t part)

That's on purpose. We're already in the REJECTED state so there's no need to check the timestamp to make sure that there was a rejection.

Actually I see that the current code does not make this distinction. Well, OK. I've changed the diagram so that all conditions for this message are identical.

[kbeker]

@cameel Actually it is not seen there. OK, as you said it's difference in rendering. So for me all is OK :)

[claassistantio]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 7 committers have signed the CLA.

✅ kbeker
❌ Pawel Kisielewicz
❌ wyspiansky87
❌ Jakub89
❌ rwrzesien
❌ cameel
❌ dybi

---

Pawel Kisielewicz seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}