search

golemcloud / golem

Golem is an open source durable computing platform that makes it easy to build and deploy highly reliable distributed systems.
https://learn.golem.cloud/
Apache License 2.0
530 stars 59 forks source link

Initial File System - Bounty-to-Hire #1032

Closed mschuwalow closed 1 week ago

mschuwalow commented 3 weeks ago

Initial Component Files

/claim #1004 /resolves #1004

Features:

Note the comment I left regarding updating workers with intial component files present. There are test present for correct behavior in automatic update mode.

For manual update I would argue that we should give control to the user and have them explicitly store / restore files from the blob storage instead of trying to do it for them.

I.e. at the time when load-snapshot gets called the user will have a fully initialized filesystem with the files declared for the new component version and can restore whatever is needed from the old files.

Alternatively, if we want to simplify this for users as much as possible, we could always restore the previous r+w files to a special, reserved descriptor in read-only mode after a manual update. I.e. something like '/@previous'. This will not show up in normal directory listings for '/' if it's added as a seperate preopen, so it should not interfere with normal operations. If desired I can add this behavior here / as a followup.

Obviously this is a very chunky boy. I tried to make it somewhat reviewable commit by commit, maybe that will help a bit.

jdegoes commented 3 weeks ago

@mschuwalow Congratulations on submitting your solution for our Bounty-to-Hire challenge (#1004)!

We have reviewed your pull request and believe that it is very promising. While not yet satisfying all of the requirements of #1004, we believe there is a short and well-defined path to get there, and so we are delighted to announce that you are a finalist in our Bounty-to-Hire event!

Next steps:

  1. You are guaranteed an interview based on your work thus far.
  2. Should you continue to work on this pull request, and get it into a state that can be merged to satisfy all requirements, you stand a reasonable chance of winning the bounty.

If you no longer wish to work on this pull request, please close it, and we will reach out about the interview. On the other hand, if you wish to continue pushing this pull request forward, then please let us know here and continue your work.

Congratulations again on being a finalist for Bounty-to-Hire!

mschuwalow commented 2 weeks ago

@vigoo could you kick off ci for me later? I would like to make sure I have no failures (and running the full suite locally sucks :D)

vigoo commented 2 weeks ago

@vigoo could you kick off ci for me later? I would like to make sure I have no failures (and running the full suite locally sucks :D)

Feel free to open a "fake" PR (changing a line in the readme or something:) that I can merge so we don't need to approve every attempt

mschuwalow commented 2 weeks ago

@vigoo could you kick off ci for me later? I would like to make sure I have no failures (and running the full suite locally sucks :D)

Feel free to open a "fake" PR (changing a line in the readme or something:) that I can merge so we don't need to approve every attempt

Looks like it's green except for some formatting stuff, will quickly fix that and leave this one alone for now. WIll follow your suggestions if I do more work on this though.

jdegoes commented 2 weeks ago

@mschuwalow

We have reached November 4th, which is the day we must announce the winner of the Bounty-to-Hire program!

Given the scope of the challenge, it's incredibly impressive that we have had not one, but THREE finalists, who each demonstrated high competence and skill in tackling a problem that would stump most engineers on the planet.

However, ultimately, there can be only ONE WINNER to the Bounty-to-Hire challenge. So after MUCH analysis by Golem engineers, thoughtful consideration, and spirited discussion, we have decided to pick a winner.

That winner will be announced on our LIVE STREAM event TODAY at 12 NOON ET:

https://www.youtube.com/watch?v=at8EPqLWIRE

Without question, all three finalists are highly qualified Rust engineers and we look forward to interviewing and getting to each of them, with the intention of making at least one job offer to fill our recently opened position.

While you await the announcement of the WINNER, please shoot an email to john@ziverge.com to schedule your interview!

Stay tuned for more.

jdegoes commented 2 weeks ago

@mschuwalow If you plan to join for the Live stream today, please do send me a message on Discord (you can find me on the Golem Discord), or an email at john@ziverge.com. Thank you!

jdegoes commented 2 weeks ago

@mschuwalow Congratulations on winning the Bounty-to-Hire event!

You will be paid directly by Algora.io when your pull request is merged.

In order to get your pull request merged:

  1. Allow time for the team to comment on necessary & suggested revisions
  2. Implement revisions until the team has no further comments
  3. Ensure that CI is green, and that all original and new tests complete successfully
  4. Notify @vigoo or other maintainer that you are ready for final review

Once merged, your $15,000 bounty will be deposited into your Algora account, where you can cash out in the ways provided to you by the platform.

Thanks for your work on this feature and congratulations on winning what turned out to be a highly competitive event!

mschuwalow commented 2 weeks ago

I think I addressed all of the comments. Please do another round of reviews :)

noise64 commented 2 weeks ago

all comments are resolved on cli side 👍

mschuwalow commented 1 week ago

@vigoo what is missing here / anything to do from my side to unblock merging?

vigoo commented 1 week ago

@vigoo what is missing here / anything to do from my side to unblock merging?

Hopefully nothing, I just want an approve from @afsalthaj too

afsalthaj commented 1 week ago

https://github.com/golemcloud/golem/pull/1032#discussion_r1832212517

@mschuwalow If you could just make this small change, I think we good to merge May be proto3 is internally optional by default, but we have been following a safer approach by making things explicitly optional to keep backward compatibility.

mschuwalow commented 1 week ago

#1032 (comment)

@mschuwalow If you could just make this small change, I think we good to merge May be proto3 is internally optional by default, but we have been following a safer approach by making things explicitly optional to keep backward compatibility.

ah sorry, missed that one. Will adjust 👍

mschuwalow commented 1 week ago

#1032 (comment)

@mschuwalow If you could just make this small change, I think we good to merge May be proto3 is internally optional by default, but we have been following a safer approach by making things explicitly optional to keep backward compatibility.

@afsalthaj Done, please take a look https://github.com/golemcloud/golem/pull/1032/commits/625786aa0dcc1d23e9250af2d5aadf2cb62011e3 Only CompiledHttpApiDefinition and ComponentMetadata seem to be getting stored as a protobuf, so the rest should be fine.

I was thinking to add a comment to all grpc definitions that (directly or transitively) get stored in the database so other people don't run into the same problem. This would affect quite a lot of types like rib expressions though (due to how big CompiledHttpApiDefinition is). Something like

// Note: This message is used to store serialized data in the database.
// make sure to not break backward compatibility when changing this message.

Want me to go for it?

afsalthaj commented 1 week ago

We are still thinking as to how to manage this: bincode vs proto. Something for us to improve. The advantage of proto is we throw the backward compatibility pollutions there. We will see.

Thanks for addressing the review comments.