)]}'
{"metropolis/node/core/curator/impl_leader_curator.go":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"29d30ca81b220a0d3689f7a7b4b53056fba294de","unresolved":true,"context_lines":[{"line_number":27,"context_line":"\t//"},{"line_number":28,"context_line":"\t// This lock has to be taken any time such RMW operation takes place when not"},{"line_number":29,"context_line":"\t// additionally guarded using etcd transactions."},{"line_number":30,"context_line":"\tmuNodes sync.Mutex"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"// Watch returns a stream of updates concerning some part of the cluster"}],"source_content_type":"text/x-go","patch_set":2,"id":"bcb286f1_db6832ab","line":30,"range":{"start_line":30,"start_character":1,"end_line":30,"end_character":8},"updated":"2021-10-04 15:35:01.000000000","message":"This is only taken by `UpdateNodeStatus` which at first glance seems odd. My understanding is that this is because currently the only way to add a node is through BootstrapFinish in the curator bootstrap (which doesn\u0027t need it because a transactional create cannot race this code). Is that correct?","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"fa3e6b0bae8a5447a7c0d9ef3e5d76fe6736306f","unresolved":false,"context_lines":[{"line_number":27,"context_line":"\t//"},{"line_number":28,"context_line":"\t// This lock has to be taken any time such RMW operation takes place when not"},{"line_number":29,"context_line":"\t// additionally guarded using etcd transactions."},{"line_number":30,"context_line":"\tmuNodes sync.Mutex"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"// Watch returns a stream of updates concerning some part of the cluster"}],"source_content_type":"text/x-go","patch_set":2,"id":"9125938d_2f422665","line":30,"range":{"start_line":30,"start_character":1,"end_line":30,"end_character":8},"in_reply_to":"b0b4e18a_0b4b891b","updated":"2021-10-05 17:16:01.000000000","message":"Ack","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"141ceb18016884805697499895bf9688c62e8147","unresolved":true,"context_lines":[{"line_number":27,"context_line":"\t//"},{"line_number":28,"context_line":"\t// This lock has to be taken any time such RMW operation takes place when not"},{"line_number":29,"context_line":"\t// additionally guarded using etcd transactions."},{"line_number":30,"context_line":"\tmuNodes sync.Mutex"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"// Watch returns a stream of updates concerning some part of the cluster"}],"source_content_type":"text/x-go","patch_set":2,"id":"b0b4e18a_0b4b891b","line":30,"range":{"start_line":30,"start_character":1,"end_line":30,"end_character":8},"in_reply_to":"bcb286f1_db6832ab","updated":"2021-10-05 16:56:12.000000000","message":"Correct. Nothing else in the leader currently modifies the nodes or even reads the nodes\u0027 status (yet).","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"29d30ca81b220a0d3689f7a7b4b53056fba294de","unresolved":true,"context_lines":[{"line_number":103,"context_line":"\t\treturn nil, status.Errorf(codes.InvalidArgument, \"Status and Status.ExternalAddress must be set\")"},{"line_number":104,"context_line":"\t}"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"\t// As we\u0027re performing a node update with two etcd transactions below (one"},{"line_number":107,"context_line":"\t// to retrieve, one to save and upate node), take a local lock to ensure"},{"line_number":108,"context_line":"\t// that we don\u0027t have a race between either two UpdateNodeStatus calls or"},{"line_number":109,"context_line":"\t// an UpdateNodeStatus call and some other mutation to the node store."},{"line_number":110,"context_line":"\tl.muNodes.Lock()"},{"line_number":111,"context_line":"\tdefer l.muNodes.Unlock()"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-go","patch_set":2,"id":"a5feaace_a97a7111","line":109,"range":{"start_line":106,"start_character":0,"end_line":109,"end_character":71},"updated":"2021-10-04 15:35:01.000000000","message":"There is technically still a race, but it is incredibly unlikely to manifest:\n1. Current curator is leader, Node gets retrieved successfully\n2. Current curator loses leadership to another curator\n3. Another curator performs an RMW since the lock is not global and it can take its own lock.\n4. Current curator regains leadership\n5. Saving the local node from 1 eliminates commited data from another curator in step 3.\n\nIn practice this is incredibly unlikely due to the fact that the amount of work done between the two transactions is miniscule. It exists because the lock is not \"invalidated\" as soon as one property its owners depend on (leadership) changes.\nMost trivial fix would be to check the revision of the key when saving against what was previously retrieved and just ABORT otherwise. But if you think it\u0027s not worth it I\u0027m fine with that.","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"fa3e6b0bae8a5447a7c0d9ef3e5d76fe6736306f","unresolved":false,"context_lines":[{"line_number":103,"context_line":"\t\treturn nil, status.Errorf(codes.InvalidArgument, \"Status and Status.ExternalAddress must be set\")"},{"line_number":104,"context_line":"\t}"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"\t// As we\u0027re performing a node update with two etcd transactions below (one"},{"line_number":107,"context_line":"\t// to retrieve, one to save and upate node), take a local lock to ensure"},{"line_number":108,"context_line":"\t// that we don\u0027t have a race between either two UpdateNodeStatus calls or"},{"line_number":109,"context_line":"\t// an UpdateNodeStatus call and some other mutation to the node store."},{"line_number":110,"context_line":"\tl.muNodes.Lock()"},{"line_number":111,"context_line":"\tdefer l.muNodes.Unlock()"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-go","patch_set":2,"id":"fe62af88_e7714207","line":109,"range":{"start_line":106,"start_character":0,"end_line":109,"end_character":71},"in_reply_to":"301ebec6_cdf3cc71","updated":"2021-10-05 17:16:01.000000000","message":"Ack, let\u0027s clean this up in a future revision.","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"141ceb18016884805697499895bf9688c62e8147","unresolved":true,"context_lines":[{"line_number":103,"context_line":"\t\treturn nil, status.Errorf(codes.InvalidArgument, \"Status and Status.ExternalAddress must be set\")"},{"line_number":104,"context_line":"\t}"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"\t// As we\u0027re performing a node update with two etcd transactions below (one"},{"line_number":107,"context_line":"\t// to retrieve, one to save and upate node), take a local lock to ensure"},{"line_number":108,"context_line":"\t// that we don\u0027t have a race between either two UpdateNodeStatus calls or"},{"line_number":109,"context_line":"\t// an UpdateNodeStatus call and some other mutation to the node store."},{"line_number":110,"context_line":"\tl.muNodes.Lock()"},{"line_number":111,"context_line":"\tdefer l.muNodes.Unlock()"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-go","patch_set":2,"id":"301ebec6_cdf3cc71","line":109,"range":{"start_line":106,"start_character":0,"end_line":109,"end_character":71},"in_reply_to":"a5feaace_a97a7111","updated":"2021-10-05 16:56:12.000000000","message":"This shouldn\u0027t happen, as the lock check in txnAsLeader will fail since it checks against a lock revision, not against a node name.\n\nHowever, this is indeed a bit ugly and I wanna move away from locks into implementing revision checks for read-then-write behaviour in txnAsLeader.","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"}],"metropolis/proto/api/configuration.proto":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"29d30ca81b220a0d3689f7a7b4b53056fba294de","unresolved":true,"context_lines":[{"line_number":18,"context_line":"package metropolis.proto.api;"},{"line_number":19,"context_line":"option go_package \u003d \"source.monogon.dev/metropolis/proto/api\";"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import \"metropolis/proto/common/common.proto\";"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"// NodeParameters is the data with which a Node is set booted. It contains the"},{"line_number":24,"context_line":"// configuration required for a node to either bootstrap a new cluster, or"}],"source_content_type":"text/x-protobuf","patch_set":2,"id":"de665540_df1c86c8","line":21,"updated":"2021-10-04 15:35:01.000000000","message":"Is that doing something? It doesn\u0027t seem to be used in this file but I\u0027m not knowledgeable enough about proto recursive imports to say so definitely.","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"141ceb18016884805697499895bf9688c62e8147","unresolved":false,"context_lines":[{"line_number":18,"context_line":"package metropolis.proto.api;"},{"line_number":19,"context_line":"option go_package \u003d \"source.monogon.dev/metropolis/proto/api\";"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import \"metropolis/proto/common/common.proto\";"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"// NodeParameters is the data with which a Node is set booted. It contains the"},{"line_number":24,"context_line":"// configuration required for a node to either bootstrap a new cluster, or"}],"source_content_type":"text/x-protobuf","patch_set":2,"id":"c0c18508_f821c8a4","line":21,"in_reply_to":"de665540_df1c86c8","updated":"2021-10-05 16:56:12.000000000","message":"Done","commit_id":"13417d6f94efeab3402075e2bceb949cf0f3295a"}]}