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

paddles: Adding a queueing mechanism to Paddles #94

Merged
merged 19 commits into from
Mar 4, 2024

Conversation

amathuria
Copy link
Contributor

This change adds support for a queuing mechanism in Paddles. The idea is to eventually use only the Paddles backend for teuthology and remove the need for the Beanstalk queue

The following are the changes made:

  1. Addition of new fields to the Jobs table in order to store the entire teuthology config.
  2. Addition of a priority field to mimic a priority queue (similar to Beanstalk)
  3. A new Queue table to keep track of the queues and their statuses on various machine types
  4. Generation of a unique Job ID in Paddles
  5. Rest APIs for retrieving a job from the queue, creating a new queue, updating status of a queue and retrieving stats for a particular queue

Signed-off-by: Aishwarya Mathuria [email protected]

@amathuria amathuria requested a review from jdurgin May 14, 2021 07:04
Copy link
Member

@jdurgin jdurgin left a comment

Choose a reason for hiding this comment

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

This is looking good! IIRC you had some alembic changes to the fields as well - looks like they weren't added in this commit.

It'd be great to add some unit tests for the new/updated models and controllers, particularly ones with logic - e.g. for updating job priority it looks like you had to modify the model to work without targets, and only an update of priority

paddles/models/jobs.py Outdated Show resolved Hide resolved
paddles/models/queue.py Outdated Show resolved Hide resolved
paddles/models/queue.py Outdated Show resolved Hide resolved
paddles/controllers/queue.py Outdated Show resolved Hide resolved
@jdurgin jdurgin requested review from zmc and kshtsk May 17, 2021 02:42
@kshtsk
Copy link
Contributor

kshtsk commented May 20, 2021

I guess we need to deploy a test instance for this.

@kshtsk
Copy link
Contributor

kshtsk commented May 20, 2021

travis tests are failing, btw

paddles/models/jobs.py Outdated Show resolved Hide resolved
@amathuria
Copy link
Contributor Author

travis tests are failing, btw

@kshtsk I can see a connection refused error in the test logs here: https://travis-ci.org/github/ceph/paddles/builds/771656065#L711. Any idea how I can fix this?

@jdurgin
Copy link
Member

jdurgin commented Jun 7, 2021

could you add a unit test for updating job priority? the new queue tests look good.

For the travis failure, do the same commands work for you locally? i.e. the commands starting here https://travis-ci.com/github/ceph/paddles/builds/227874146#L196

Though travis doesn't show the paddles log, it looks like paddles is crashing quickly after startup

@kshtsk
Copy link
Contributor

kshtsk commented Jun 7, 2021

travis tests are failing, btw

@kshtsk I can see a connection refused error in the test logs here: https://travis-ci.org/github/ceph/paddles/builds/771656065#L711. Any idea how I can fix this?

This means that server is not started in fact, you must try reproduce it on your local machine.

@kshtsk
Copy link
Contributor

kshtsk commented Jun 9, 2021

E           TypeError: Invalid argument(s) 'listeners' sent to create_engine(), using configuration SQLiteDialect_pysqlite/NullPool/Engine.  Please check that the keyword arguments are appropriate for this combination of components.

@kshtsk
Copy link
Contributor

kshtsk commented Jun 9, 2021

paddles/models/__init__.py:124:1: F401 '.queue.Queue' imported but unused
paddles/models/queue.py:1:1: F401 'sqlalchemy.exc' imported but unused

@amathuria
Copy link
Contributor Author

travis tests are failing, btw

@kshtsk I can see a connection refused error in the test logs here: https://travis-ci.org/github/ceph/paddles/builds/771656065#L711. Any idea how I can fix this?

This means that server is not started in fact, you must try reproduce it on your local machine.

Sure I will do that.

@kshtsk
Copy link
Contributor

kshtsk commented Jun 9, 2021

> docker run --name=xxxx -d -p 80:8080 -e PADDLES_SERVER_HOST=0.0.0.0 -e PADDLES_SQLALCHEMY_URL=sqlite:///dev.db ceph-paddles-test
9215e5fe3b5bb24e433b6ac2586df37523fcae114497b2495fccfb1c20615949
> docker logs xxxx
==> LOADING ENVIRONMENT
Traceback (most recent call last):
  File "/usr/local/bin/pecan", line 11, in <module>
    sys.exit(CommandRunner.handle_command_line())
  File "/usr/local/lib/python3.6/dist-packages/pecan/commands/base.py", line 96, in handle_command_line
    runner.run(sys.argv[1:])
  File "/usr/local/lib/python3.6/dist-packages/pecan/commands/base.py", line 91, in run
    self.commands[ns.command_name]().run(ns)
  File "/usr/local/lib/python3.6/dist-packages/paddles/commands/populate.py", line 20, in run
    self.load_app()
  File "/usr/local/lib/python3.6/dist-packages/pecan/commands/base.py", line 162, in load_app
    return load_app(self.args.config_file)
  File "/usr/local/lib/python3.6/dist-packages/pecan/core.py", line 218, in load_app
    app = module.app.setup_app(_runtime_conf, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/paddles/app.py", line 7, in setup_app
    models.init_model()
  File "/usr/local/lib/python3.6/dist-packages/paddles/models/__init__.py", line 74, in init_model
    conf.sqlalchemy.engine = _engine_from_config(conf.sqlalchemy)
  File "/usr/local/lib/python3.6/dist-packages/paddles/models/__init__.py", line 86, in _engine_from_config
    return create_engine(url, **configuration)
  File "<string>", line 2, in create_engine
  File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/util/deprecations.py", line 298, in warned
    return fn(*args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/sqlalchemy/engine/create.py", line 645, in create_engine
    engineclass.__name__,
TypeError: Invalid argument(s) 'listeners' sent to create_engine(), using configuration SQLiteDialect_pysqlite/NullPool/Engine.  Please check that the keyword arguments are appropriate for this combination of components.

@kshtsk
Copy link
Contributor

kshtsk commented Jun 9, 2021

@amathuria usually you must make the PR run without unittests failures first ;-)

paddles/controllers/queue.py Outdated Show resolved Hide resolved
@amathuria
Copy link
Contributor Author

amathuria commented Jun 9, 2021

@amathuria usually you must make the PR run without unittests failures first ;-)

Thank you for the logs. I was testing my changes on a mira setup with postgres and tested the sqlalchemy library upgrade there (which is causing the issue here in the docker test) and not with sqlite so I missed this. I'm working on the fix.

@jdurgin
Copy link
Member

jdurgin commented Jul 19, 2021

replace beanstalk queue

kamoltat added a commit to ceph/teuthology that referenced this pull request Aug 4, 2021
Developers can now use start.sh to build the images
and set up postgresql, paddles, pulpito and teuthology
for development.

This PR is also pending for:
#1650
ceph/paddles#94

to be merged, as currently we use these branches
as images for paddles and pulpito.

Signed-off-by: Kamoltat Sirivadhna <[email protected]>
paddles/models/jobs.py Outdated Show resolved Hide resolved
kamoltat added a commit to ceph/teuthology that referenced this pull request Sep 16, 2021
Developers can now use start.sh to build the images
and set up postgresql, paddles, pulpito and teuthology
for development.

This PR is also pending for:
#1650
ceph/paddles#94

to be merged, as currently we use these branches
as images for paddles and pulpito.

Signed-off-by: Kamoltat Sirivadhna <[email protected]>
@amathuria amathuria force-pushed the wip-amathuria-removing-beanstalkd branch 2 times, most recently from 855eeac to a2eb61d Compare October 28, 2021 15:12
Copy link
Member

@zmc zmc left a comment

Choose a reason for hiding this comment

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

There's a lot of good work here! I don't think this is too far from being ready.

I've left a lot of comments, and tried to be clear, but if you have any questions, or if I just got something wrong, let me know.

paddles/models/jobs.py Outdated Show resolved Hide resolved
paddles/controllers/jobs.py Outdated Show resolved Hide resolved
paddles/models/jobs.py Outdated Show resolved Hide resolved
paddles/controllers/queue.py Show resolved Hide resolved
paddles/controllers/queue.py Outdated Show resolved Hide resolved
paddles/controllers/jobs.py Outdated Show resolved Hide resolved
paddles/controllers/jobs.py Outdated Show resolved Hide resolved
paddles/controllers/jobs.py Outdated Show resolved Hide resolved
paddles/controllers/queue.py Outdated Show resolved Hide resolved
paddles/controllers/root.py Outdated Show resolved Hide resolved
@amathuria amathuria force-pushed the wip-amathuria-removing-beanstalkd branch 3 times, most recently from 6bca229 to 8f10855 Compare October 18, 2023 04:47
@amathuria amathuria force-pushed the wip-amathuria-removing-beanstalkd branch from 8f10855 to d04e477 Compare November 22, 2023 09:57
This change adds support for a queuing mechanism in Paddles. The idea is to eventually use only the Paddles backend for teuthology and remove the need for the Beanstalk queue

The following are the changes made:
1. Addition of new fields to the Jobs table in order to store the entire teuthology config.
2. Addition of a priority field to mimic a priority queue (similar to Beanstalk)
3. A new Queue table to keep track of the queues and their statuses on various machine types
4. Generation of a unique Job ID in Paddles
5. Rest APIs for retrieving a job from the queue, creating a new queue, updating status of a queue and  retrieving stats for a particular queue

Signed-off-by: Aishwarya Mathuria <[email protected]>
Signed-off-by: Aishwarya Mathuria <[email protected]>
1. Adding error message for "user" key not found
2. Removing explicit rollbacks

Signed-off-by: Aishwarya Mathuria <[email protected]>
Adding run name as a filter for retrieving jobs from paddles

Signed-off-by: Aishwarya Mathuria <[email protected]>
1. listeners option in configuration is now deprecated. Using event.listen instead.
2. Adding alternative to using Sequence() since it is not supported in SQLite
1. listeners option in configuration is now deprecated. Using event.listen instead.
2. Adding alternative to using Sequence() since it is not supported in SQLite
3. Modifying test cases for SQLite testing
As we are adding a few columns to the Jobs table this script will take care of the upgrade of the table schema

Signed-off-by: Aishwarya Mathuria <[email protected]>
Signed-off-by: Aishwarya Mathuria <[email protected]>
1. pop_queue only is queue is not paused
2. retrieve queued_jobs with just the queue name, user and run_name are optional parameters
3. addition of queue column to the Job schema
4. simplify queue pausing mechanism with the addition of paused_until

Signed-off-by: Aishwarya Mathuria <[email protected]>
In order to store the job configuration details in Paddles we need to
add a few columns to the existing Jobs table

Signed-off-by: Aishwarya Mathuria <[email protected]>
In order to store the job configuration details in Paddles we need to
add a few columns to the existing Jobs table

Signed-off-by: Aishwarya Mathuria <[email protected]>
Due to a mismatch in name, post requests were getting sent to a function meant to handle only PUT requests.

Signed-off-by: Aishwarya Mathuria <[email protected]>
@amathuria amathuria force-pushed the wip-amathuria-removing-beanstalkd branch from d04e477 to 4ad5614 Compare November 29, 2023 15:36
@amathuria amathuria force-pushed the wip-amathuria-removing-beanstalkd branch from a07f537 to 4ad5614 Compare December 6, 2023 16:11
Signed-off-by: Aishwarya Mathuria <[email protected]>
@amathuria amathuria force-pushed the wip-amathuria-removing-beanstalkd branch from c6e562a to bcc6b4e Compare February 28, 2024 16:06
@zmc zmc merged commit 27576bd into ceph:main Mar 4, 2024
3 checks passed
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.

4 participants