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 an auto-incremental id to jobs #38

Closed
josecelano opened this issue Feb 11, 2022 · 13 comments · Fixed by #179
Closed

Add an auto-incremental id to jobs #38

josecelano opened this issue Feb 11, 2022 · 13 comments · Fixed by #179
Assignees
Labels
enhancement New feature or request

Comments

@josecelano
Copy link
Member

As we defined in this issue we want to give the job an auto-increment id.

Some examples of commit subject using the job id.

📝🈺: {QUEUE_NAME}: job.{JOB_NUMBER}
📝🈺: update_artwork: job.24
📝👔: {QUEUE_NAME}: job.{#JOB_POSITION}/{#PENDING_JOBS}: job.ref.{JOB_CREATION_COMMIT_HASH}
📝👔: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝👕: {QUEUE_NAME}: job.{#JOB_POSITION}/{PENDING_JOBS}: job.ref.{JOB_CREATION_COMMIT_HASH}
📝👕: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490

We can implement this feature before allowing the queue to have more than one pending job.

@yeraydavidrodriguez
Copy link
Collaborator

#Non-job queue elements

A feature that will be included in this same issue are the events to initialize / deinitialize (and potentially, create / destroy) queues.

Currently, the only command accepted by the actions are:

  • Create a job
  • Start a job
  • Finish a job

However, we are going to introduce those new commands:

  • Deactivate a queue
  • Activate a queue (in case that was previously deactivated, re-activate it)

A queue that is deactivated (not active) will not accept new jobs. Each time a queue is activated, the counter that assigns numbers to jobs is reset. That means that, inside a queue, several jobs can share the same ID, but NOT inside an activate-deactivate span. When the first job of a queue is created, an automatic "queue_name_activated" message is created.

Those new events will be marked inside the queue as new messages, with their corresponding commits. The commit messages for those events could be (and is just a proposal):

📝▶️: queue_name activated
📝⏸: queue_name deactivated

sample of how a simplified git history (showing only commit messaged) could be:

...
📝🈺: update_artwork: job.id.1 job.ref.ddc123312232baa3acb2342562fbc4535ccc234
📝▶️: update_artwork: activated
📝⏸: update_artwork: deactivated
📝✅: update_artwork: job.id.2: job.ref.232baa3acb2342562fbc4535ccc234ddc123312
📝👔: update_artwork: job.id.2: job.ref.232baa3acb2342562fbc4535ccc234ddc123312
📝🈺: update_artwork: job.id.2: job.ref.232baa3acb2342562fbc4535ccc234ddc123312
📝✅: update_artwork: job.id.1: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝👔: update_artwork: job.id.1: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝🈺: update_artwork: job.id.1: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝▶️: update_artwork: activated

About the ids

A proposal was made by @da2ce7 to mark the "activated" message with ID 0 (zero). However, as discussed with Jose Celano, this could lead to some UX problems:

  • Checking the git-queue generated commits, it gets intuitively clear for the user that the IDs are assigned to JOBs.
  • If an ID is also set to a "queue activated" event (message), this will lead the user to think initially that the IDs are assigned to commit messages, not jobs. However, all messages assigned to a job share the same ID, contradicting this hypothesis.
  • So from the user perspective, as both jobs and "queue control events" has consecutive ID, both belong to the same sequence, and therefor are specificities of the same abstraction: a kind of "queue element" class, from which jobs and "queue control events" inherit.
  • This implies that we have to introduce two new elements in the system: that new "queue element" class with the ID, and a "queue control element", with a linked message (jobs would have three linked messages).

This would make the system bigger, more complex and more obscure. Although I understand the concerns of a sequence of IDs beginning from nowhere, I can consider that the user will easily understand that the git activation event is the root of the sequence, without needing to explicitly assign IDs to "queue control events" elements. But of course I may be wrong :)

Please @josecelano @da2ce7, let me know your thoughts about this.

@josecelano
Copy link
Member Author

I think some names would be surprising for newcomers:

  1. I would expect something to exist before activate it. If you activate a queue it sounds like it should exist before.
  2. For me it would be surprising if job numbers restart after deactivate and activate a queue. In that case, I would call the action reset or even more explicitly restart-numbering.
  3. With the new messages I do not know why we have to add the keywords activated and deactivate. We decided to use emojis for the message keys. For me the emoji and the keyword are redundant.

I would use this new message:

📝▶️: update_artwork

like start-queue action.

And the other one:

📝⏸: update_artwork

like pause-queue action to stop accepting new jobs.

And I would add an extra message reset-job-numbering if we want to restart the jobs IDs.

In the start-queue action we can add a metadata field job-starting-index: 1 to indicate how we are going to number jobs.

...
📝🈺: update_artwork: job.id.1 job.ref.ddc123312232baa3acb2342562fbc4535ccc234
📝▶️: update_artwork
📝⏸: update_artwork
📝✅: update_artwork: job.id.2: job.ref.232baa3acb2342562fbc4535ccc234ddc123312
📝👔: update_artwork: job.id.2: job.ref.232baa3acb2342562fbc4535ccc234ddc123312
📝🈺: update_artwork: job.id.2: job.ref.232baa3acb2342562fbc4535ccc234ddc123312
📝✅: update_artwork: job.id.1: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝👔: update_artwork: job.id.1: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝🈺: update_artwork: job.id.1: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
📝▶️: update_artwork

The workflows should use these actions:

  • start-queue
  • create-job
  • start-job
  • finish-job
  • pause-queue
  • reset-job-numbering

If we want to be more explicit regarding job numbering in the commit subject I would do something like:

📝▶️: update_artwork job.sequence.start.1

If we want to be explicit regarding the last job number after reset-job-numbering action we can also include a job.sequence.end.42 field to the commit subject of a finish-queue action or reset-job-numbering action.

📝🔄: update_artwork job.secuence.end.42

In general, I do not like resetting the job sequence. If the only reason to allow job sequence reset is to allow reusing the queue name, I would also g solve that problem by using the queue commit hash like the real queue ID instead of the name. As we discussed all jobs could have a metadata field with the queue creation (starting) commit. We could still use the queue name to reference it because we guarantee queues are not nested.

Finally, I'm not sure if using commit hashes it's a good idea for both linking job messages and identifying queues. If we "rebase", commit hashes change. We have been pushing directly to the main branch but for example if we want to create unsigned commits and signed them later, the commit hash is going to change. Maybe we should also number messages and use the message ID to reference other messages.

@yeraydavidrodriguez
Copy link
Collaborator

3. We decided to use emojis for the message keys. For me the emoji and the keyword are redundant.

Personally I don't think so. I'm not sure that we will find an emoji expressive enough to replace the action name. Actually, I find that also previously defined actions (job create, start and finish) should also have their names (without the "job" word) in the message.

They are not redundant because they serve different purposes:

  • Emojis are for very fast browsing. A colorful and very prominent visual hint, like the website icons in the tabs of the browser.
  • Words are a complement for the many situations in which the emojis does not provide all required meaning.

@yeraydavidrodriguez
Copy link
Collaborator

And I would add an extra message reset-job-numbering if we want to restart the jobs IDs.

I agree: probably resetting the IDs after a queue is deactivated/paused is not intuitive. Also, there are lots of situation in which we want to pause the queue WITHOUT resetting the numbers (e.g. the output being consumed by another system that expects unique IDs in all the queue, not only active-deactive spans). Having the reset effect moved to another action we can solve both scenarios.

@josecelano
Copy link
Member Author

  1. We decided to use emojis for the message keys. For me the emoji and the keyword are redundant.

Personally I don't think so. I'm not sure that we will find an emoji expressive enough to replace the action name. Actually, I find that also previously defined actions (job create, start and finish) should also have their names (without the "job" word) in the message.

They are not redundant because they serve different purposes:

  • Emojis are for very fast browsing. A colorful and very prominent visual hint, like the website icons in the tabs of the browser.
  • Words are a complement for the many situations in which the emojis does not provide all required meaning.

OK, maybe the problem is we are considering the commit subject as both a UI and a state store.
From the UX perspective is not a duplication. From an easy-to-parse and data-store format view, it would be convenient not to have those duplicates. I guess in this case UX has more importance because we can include a more formal data structure in the body with the JSON schema.

If we agree on this @yeraydavidrodriguez and @da2ce7, we should add it to the documentation and be always sure:

  • We include all the data inside the body, even if it's already present on the subject. That would make it possible to iterate over all commit messages and get all the info using only the body.
  • Anyway, we can define a strict format for the subject. That reminds me we do not have either a formal schema or version for the commit subject. We are only using the scheme for the commit body. And the subject could also change. Maybe we should create a schema also for that part and include the version on it. I do not think we can use the same schema because this part is not using a JSON format. The commit subject is as much likely to change as the body.

@yeraydavidrodriguez
Copy link
Collaborator

"OK, maybe the problem is we are considering the commit subject as both a UI and a state store."

You are right, this duality is somehow limiting. Maybe we should change our perspective and consider the subject as a UX-only entity, with an user-friendly print of the relevant information, and do the necessary parsing only in the body.

@da2ce7
Copy link
Contributor

da2ce7 commented May 3, 2022 via email

@da2ce7
Copy link
Contributor

da2ce7 commented May 3, 2022

  1. Queue Allocation and Deallocation
    A Queue may have multiple staring points, by different authors on different forks, that need to be reconciled before merging.

@josecelano and @yeraydavidrodriguez some more background for my reasoning.

With Git-Queue, the Git Repository is the synchronization authority. I.e. We must not depend on any other external oracle for synchronization.

However, Git is a Decentralized Data Structure. So it may be forked at any point, where each fork is extended interdependently. For git, and it's commit histories, the point of resynchronization, is the 'merge', a special commit that can reference one, two, or many branched histories and bring them together.


We have to cover the two cases:

  1. An Active Git-Queue is branched, the active queue is cloned, and from that point, the job history diverges.
  2. Multiple independent Git-Queues allocated on different branches, and never share a common starting ancestor, while sharing the same queue name.

We should detect if there is going to be a git-queue merge conflict.

To resolve this conflict we have two options.

  1. Finish/Close/Delete any outstanding jobs, and then deallocate the queue on each branch before merging. Merge. Allocate a new Queue to replace the queue deallocated before the merge.
  2. If the queues do not share a common ancestor, then we could select one to the be 'main', and only deallocate all the others.

As a note, if the queue was branched, I don't think that it is elegant to have a 'partly deallocated' queue. We should deallocate all queues that share a common ancestor before merging branches of that queue.

We could also decide that it is best to deallocate all the conflicting queues, even if they do not share an ancestor.

@josecelano
Copy link
Member Author

hi @da2ce7

  1. Merging and Rebasing.

Commits should be merged into the upstream. Keeping the history and signatures intact.
Downstream branches should be rebased upon the upstream.
Rebases should be preformed by the commit author.

So the final processor would be:

  • Contributor forks the repo.
  • Contributor creates a new branch issue-xxxx-description in the fork.
  • Contributor creates commits and push to the branch in the fork.
  • Contributor creates a PR into main in the upstream repo.

If it's all ok we merge with:

git checkout main
git pull
git checkout -b contributor-fork-issue-xxxx-description main
git pull https://github.com/contributor/git-queue.git issue-xxxx-description
git checkout main
merge --ff-only contributor-fork-issue-xxxx-description
git push origin main

If there are new commis in the main branch:

  • Contributor has to rebase their branch (signing new commits with their GPGP keys).
  • Go back to the previous state and merge.

The commit is signed with the Nautilus bot GPG key and shown as partially verified.

With git merge --ff-only we would not lose the contributor's commit signature if we did not use the bot signature. Right now we get partially verified commits because the commit changes because we sign it again with a different GPG key (and we change the committer).

  1. Rebasing for signing by the author / committer.

This is a very natural. Often the author would prefer to keep their signing keys on a different environment from their development. Also, many authors may prefer or need to submit patches via email or other out-of-git formats, the committer should sign them without being the commit author.

OK, but we are going to lose the signature anyway unless we merge with --no-ff option. As far as I know, we have to use FF if we want to solve the original concurrency problem. WE detect concurrency conflict in the git-queue not with a contents merge conflict but by forcing a merge conflict when a FF is not possible. I mean, if we do not use FF we could merge for example, two jobs with the same ID, after running two "work-allocator" workflows in parallel, couldn't we?

  1. Rebasing Changes the Commit Hashes.

When using a rebase, the commit id‘s are updated, this should be kept track of, and the corresponding id references should be updated in the child commits.

So, I suppose we should provide a new "action" (command) for the action to do that, right?
And that nation will be used in the script for emerging into main and also by the contributor. I do not know if that should be a new command. The action is used in workflows not directly as a console command. I'm not sure if that should be part of the action. The "merge script" should be an independent project but this "queue rebase" command maybe should be a console command to use the queue. Maybe we have to remove the core logic form the action and have 3 repos: the core package, the console app, and the GitHub Action. For the time being, I can use the GitHub Action as a console app using env vars.

  1. Queue Allocation and Deallocation

A Queue may have multiple staring points, by different authors on different forks, that need to be reconciled before merging.
The simple option is to deallocate all the conflicting queues in their parent branches before merging. Then allocating the queue again post-merge.
When deleting (deallocating) a queue, we first delete any outstanding jobs, then delete the queue.

I understand the problem of having:

  • Forked queues.
  • "Subqueues" in forks.

But before discussing problems/solutions I need to know what you mean by "allocation" and "deallocation" in terms of the git queue commits.

  1. Job Numbers.

When a queue is newly allocated the job number is zero. Meaning there is no jobs for this queue yet.

In terms of implementation, does that mean the action would have these "actions"?:

  • allocate-queue
  • create-job
  • start-job
  • finish-job
  • pause-queue
  • reset-job-numbering
  • deallocate-queue

The first command allocate-queue creates a job with ID 0 that is not an internal queue job (control job). It's also consumed by the user, right?

@josecelano
Copy link
Member Author

josecelano commented May 11, 2022

Hi @da2ce7,

@yeraydavidrodriguez @ivanramosnet and I have been thinking about your points (1 to 5). We think each of them could have its own discussion but I think we are mixing up some topics. I would suggest re-organizing the discussions into these ones:

  1. How to merge PR (from contributors).
  2. How to merge queue commits with queue messages (from bots).
  3. How to upgrade message references to commit hashes after a git rebase.
  4. Queue Allocation and Deallocation.
  5. Re-define the queue API (if we add automatically generated jobs).

In our next meeting, we would like to start discussing point 4, because we think it's a blocker. We could even reconsider whether git-queue makes sense or not. We do not find an easy solution to re-write references from the user's point of view. It forces the user to either:

  • Not to use rebase.
  • Use qit-queue or other custom git rebase wrapper to rebase.

In more detail we think these are the key points for each discussion:

1. How to merge PR (from contributors).

  • Contributors (not bots) create a PR.
  • That's the process I described in my previous comment.
  • What happens with commits signatures.
  • If we use git merge we keep the contributor signature.
  • I suppose we do not need them to force the FF.
  • In case we need to rebase the PR branches, that has to be done by the contributor in order to keep their commit signatures.

2. How to merge queue commits with queue messages (from bots).

Be aware that we were discussing two solutions but we have only implemented one:

  • A) git-queue only uses the main branch. work-allocator and worker directly push from the main branch in the runner into the main branch in the origin repo.
  • B) git-queue creates new PRs and merges the new queue commits later into main. For the time being, we discarded this option because it's too complex. We would probably need a GitHub App.

Considering we have implemented option A), our solution is based on:

  • Having a conflict when you try to push to main and main has changed, so you need to rebase or force push.

With option B) we would need:

  • To use only fast-forward merges.
  • If we do not want to lose the original signature, we need to rebase in the PR branch before merging (rebase done by the author) and merge using git merge --ff-only. This is why I said we were mixing up things. git merge --ff-only is only needed if we were using PR to integrate queue commits.

3. How to upgrade message references to commit hashes after a git rebase.

This is a very complex problem. We see two obvious solutions:

A) We do not use commit hashes in the commit subject or body. What else can we use? We do not know, maybe the Job id. But that would make searches slow.
B) We write a kind of wrapper for git rebase because all rebase including git-queue commits should be rebased using the wrapper. We need to know the internal queue logic in order to change the subject and body.

4. Queue Allocation and Deallocation.

We think we have to define first the scope and all use cases. You started doing it here, but I would like to specify how that log affects the API. I mean, if the user has to explicitly allocate/deallocate the queues, etcetera.

5. Re-define the queue API (if we add automatically generated jobs).

Regarding this topic, we can continue the discussion on the issue because it was related to this WIP issue.

If we create those "control jobs" we need to know if those jobs are going to be consumed by the end-user.

@josecelano
Copy link
Member Author

josecelano commented May 13, 2022

Yesterday we had a meeting (@da2ce7, @ivanramosnet, @yeraydavidrodriguez, @cgbosse, @josecelano ) and we decided:

  1. We should stop using commit hashes to link queue messages. Commits are immutable, but when you re-create a commit (rebase or amend) you have a new commit with a different hash. That makes it very hard to keep references between messages. Either you write a wrapper for Git rebase command (to update references) or you do not allow the use of the rebase command when you have queue commits.

We were discussing some solutions but before that, we define the types of links we could have in the queue:

1.1 Link queue messages. Some messages reference previous messages. For example, the message JobFinishedMessage contains a link to the JobStartedMessage where that job was started.
1.2 Link jobs. Some jobs could have dependencies. Right now the queue does not handle those dependencies. The queue user has to add such information in the job payload. For example, if they want to link Job 3 to Job 1, the user can add that information to the job payload. GitQueue only guarantees the execution of jobs in order.

Solution:

1.A) @da2ce7 proposed not to use links at all. For example, the JobFinishedMessage could contain a copy of the JobStartedMessage. That means we change the relationship strategy from "link" to "embed".
2.A) @josecelano proposed to use a message autoincrement ID. Messages are linked using the message ID.

Any solution where we do not use commit hashes has a side effect, we need to search for messages in the Git history. We lose the index. If a user wants to search for a git-queue message they need a git-queue command. With solution 1.A) the last job message would contain the whole job history. But if you want to search for the JobStartedMessage message we should write a console command to do something like git-queue find 👔 4 (find JobStartedMessage for job 4).

  1. Regarding queue allocation and deallocation we agree there are two different scenarios:

2.1 You need to create the first queue message in a given branch. There is no previous commit for the new queue.

2.2 The queue already has commits, and the branch where those commits are is going to be merged into another branch.
We did not discuss the names (whether we should use the same name or not, or change the names). But the second case is harder because it implies rewriting commits (and their internal links).

  1. We also mentioned that we were planning not to sign git queue commits and let the queue user do it after all new commits are created. That case forces us to rebuild the queue message links always.

Conclusions

There are two big changes:

  • We have to re-implement how we store and link messages.
  • @da2ce7 also proposed to re-think the queue design and also take a look at the io_uring way of doing asynchronous tasks. It has 2 ring buffers, one for submission of requests (submission queue or SQ) and the other that informs you about the completion of those requests (completion queue or CQ).
  • It seems the new design implies having a console command to perform some operations on the queue.

That led us to think we need to implement a new git-queue concept from the scratch probably as a console command. The console command could have a GitHub Action wrapper in order to make it easier to use the app in a GitHub workflow. Anyway, we knew that at some point it could be useful to extract the git-queue core package into its own NPM package.

Next steps

  1. Create some new discussion in this repo just to keep track of the 5 points we described here. Those topics are going to be relevant even for the new projects.
  2. We finish the release of git-queue 1.0.0. It's useful in some contexts like in the projects where we are using it. We have to update the documentation with the requirements/constraints. The biggest one is you can not re-create queue commits and you have to fix queue conflicts by yourself. If you have the queue only in one branch (for example main), you push directly from workflows (no PR) and you mess up queue names, it should be OK.
  3. We start a new project git-queue and rename this one to git-queue-gh-action. We already have some tasks:
    3.1 Learn more about queues theory.
    3.2 Learn about io_uring.
    3.3 Decide which language/framework/etcetera would be the best option for this new app.

Notes

  • Just for clarification, we have a job queue where we are using event sourcing to store Job entities state. And the event store is git empty commits.
  • A Git empty commit is a commit that has the exact same tree as its sole parent commit. That means no changes were added to the repo.

@josecelano
Copy link
Member Author

josecelano commented May 13, 2022

Extract discussion: How to merge PRs created by developers

@josecelano
Copy link
Member Author

Extract discussion: How to keep queue consistence.

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

Successfully merging a pull request may close this issue.

3 participants