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

Add support for metadata in execution results #157

Merged
merged 5 commits into from
Jul 4, 2024

Conversation

Maelkum
Copy link
Collaborator

@Maelkum Maelkum commented Jul 1, 2024

This PR is a revamp of #156 and #154 that makes not-so-big structural changes, fixes a few bugs and implements some review suggestions.

In order to move things faster and not block the process for too long, I opened this PR instead of going back and forth there, even though the changes aren't huge.

Summary of changes compared to the mentioned PR:

  • executor is clean and agnostic of any metadata stuff - its only concern is process execution (harvesting outputs, resource limits / collecting resource usage etc)
  • aggregation works correctly - we keep track of per-node metadata and return it alongside the results
  • direct executions, raft and pbft all produce metadata if the node is configured to produce it
  • PBFT fixed - execution outputs need to be compared but metadata and resource usage ignored

Sidenote - not using metadata changes nothing compared to what we had before.

To use metadata one only needs to add node.WithMetadataProvider() config option to the node constructor.

Sample output of an API response with four nodes is below. The metadata presented is some gibberish just to illustrate the point, and one out of four nodes produces a dummy output just to showcase differing results in the output and how the metadata is moved alongside the relevant execution result.

{
    "cluster": {
        "peers": [
            "12D3KooWPSGR8UD4wv2cTJexTu1CtUTmsK68WK8YKmtdokPQDhp2",
            "12D3KooWNsALVYngvrherMatRrbJFTBxVnexEshiuKcL7vXtdQJR",
            "12D3KooWEmzWSh7FRTAVtRXdKAT3yoLJn49fp11kZYxAE6h3kwvh",
            "12D3KooWFNprYL4S13FWc2jM2GwbjH614SRAeeJ8Nr8agVkfCbgY"
        ]
    },
    "code": "200",
    "request_id": "6222d137-2601-44e2-9be0-229dd43e6e7f",
    "results": [
        {
            "result": {
                "stdout": "Hello, world!\n",
                "stderr": "",
                "exit_code": 0
            },
            "peers": [
                "12D3KooWNsALVYngvrherMatRrbJFTBxVnexEshiuKcL7vXtdQJR",
                "12D3KooWFNprYL4S13FWc2jM2GwbjH614SRAeeJ8Nr8agVkfCbgY",
                "12D3KooWPSGR8UD4wv2cTJexTu1CtUTmsK68WK8YKmtdokPQDhp2"
            ],
            "metadata": {
                "12D3KooWFNprYL4S13FWc2jM2GwbjH614SRAeeJ8Nr8agVkfCbgY": {
                    "function_id": "bafybeia24v4czavtpjv2co3j54o4a5ztduqcpyyinerjgncx7s2s22s7ea",
                    "now": "2024-07-01 22:20:26.434380767 +0200 CEST m=+12.028540649",
                    "value": "303678897189949964"
                },
                "12D3KooWNsALVYngvrherMatRrbJFTBxVnexEshiuKcL7vXtdQJR": {
                    "function_id": "bafybeia24v4czavtpjv2co3j54o4a5ztduqcpyyinerjgncx7s2s22s7ea",
                    "now": "2024-07-01 22:20:26.420332367 +0200 CEST m=+12.010193849",
                    "value": "7676128847936803138"
                },
                "12D3KooWPSGR8UD4wv2cTJexTu1CtUTmsK68WK8YKmtdokPQDhp2": {
                    "function_id": "bafybeia24v4czavtpjv2co3j54o4a5ztduqcpyyinerjgncx7s2s22s7ea",
                    "now": "2024-07-01 22:20:26.404062668 +0200 CEST m=+11.995925750",
                    "value": "3701576779347870784"
                }
            },
            "frequency": 75
        },
        {
            "result": {
                "stdout": "dummy fake response",
                "stderr": "",
                "exit_code": 0
            },
            "peers": [
                "12D3KooWEmzWSh7FRTAVtRXdKAT3yoLJn49fp11kZYxAE6h3kwvh"
            ],
            "metadata": {
                "12D3KooWEmzWSh7FRTAVtRXdKAT3yoLJn49fp11kZYxAE6h3kwvh": {
                    "function_id": "bafybeia24v4czavtpjv2co3j54o4a5ztduqcpyyinerjgncx7s2s22s7ea",
                    "now": "2024-07-01 22:20:26.426626267 +0200 CEST m=+12.014175849",
                    "value": "9178016495582307733"
                }
            },
            "frequency": 25
        }
    ]
}

@Maelkum Maelkum requested review from dmikey and zees-dev July 1, 2024 21:09
@Maelkum Maelkum self-assigned this Jul 1, 2024
Copy link
Collaborator

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Maelkum Maelkum merged commit 75fcc19 into main Jul 4, 2024
5 checks passed
@Maelkum Maelkum deleted the feat/worker-response-metadata-support-revamp branch July 4, 2024 09:24
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