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

Docs: Add succint overview of limitations of no-services profile #6537

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Jul 18, 2024

A table is added to the quick installation guide that gives a more succinct overview of the limitations of a profile created without a broker and with SQLite instead of PostgreSQL. This was already explained in more detail in text, but that was not complete and could be too dense for users, making them likely to skip it.

The code is updated to provide a link to this documentation section whenever an error is displayed that a broker is not configured. This way users can try to understand why the functionality they are trying to use is not supported and how, if they really care about it, they can go about still installing and configuring RabbitMQ after the fact.

@sphuber
Copy link
Contributor Author

sphuber commented Jul 18, 2024

@giovannipizzi @mbercx @khsrali @GeigerJ2 @superstar54 @agoscinski @unkcpz This is a first draft for improving the docs with an overview of limitations of no-broker and SQLite-vs-PostgreSQL. Comments are welcome but please make sure that you read the entire installation section if you haven't done so already. Just to have the correct context of what may be explained elsewhere.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 77.82%. Comparing base (ef60b66) to head (6299874).
Report is 118 commits behind head on main.

Files with missing lines Patch % Lines
src/aiida/cmdline/commands/cmd_process.py 50.00% 1 Missing ⚠️
src/aiida/cmdline/utils/decorators.py 66.67% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6537      +/-   ##
==========================================
+ Coverage   77.51%   77.82%   +0.32%     
==========================================
  Files         560      565       +5     
  Lines       41444    41937     +493     
==========================================
+ Hits        32120    32634     +514     
+ Misses       9324     9303      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 49 to 69
By default, ``verdi presto`` creates a profile that uses SQLite instead of PostgreSQL and does not use the RabbitMQ message broker.
The table below gives a quick overview of the functionality that is not supported in those cases:

+-----------------------------------------+------------------------------------------------------------------------+
| No RabbitMQ | SQLite instead of PostgreSQL |
+=========================================+========================================================================+
| Cannot run the daemon | Not suitable for high-throughput workloads |
+-----------------------------------------+------------------------------------------------------------------------+
| Cannot submit processes to the daemon\* | No support for ``has_key`` and ``contains`` operators in query builder |
+-----------------------------------------+------------------------------------------------------------------------+
| Cannot play, pause, kill processes | No support for ``QueryBuilder.get_creation_statistics`` |
+-----------------------------------------+------------------------------------------------------------------------+

\* Calculations can still be run on remote computers

.. note::
To enable the RabbitMQ broker for an existing profile, :ref:`install RabbitMQ <installation:guide-complete:rabbitmq>` and then run ``verdi profile configure-rabbitmq``.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default, ``verdi presto`` creates a profile that uses SQLite instead of PostgreSQL and does not use the RabbitMQ message broker.
The table below gives a quick overview of the functionality that is not supported in those cases:
+-----------------------------------------+------------------------------------------------------------------------+
| No RabbitMQ | SQLite instead of PostgreSQL |
+=========================================+========================================================================+
| Cannot run the daemon | Not suitable for high-throughput workloads |
+-----------------------------------------+------------------------------------------------------------------------+
| Cannot submit processes to the daemon\* | No support for ``has_key`` and ``contains`` operators in query builder |
+-----------------------------------------+------------------------------------------------------------------------+
| Cannot play, pause, kill processes | No support for ``QueryBuilder.get_creation_statistics`` |
+-----------------------------------------+------------------------------------------------------------------------+
\* Calculations can still be run on remote computers
.. note::
To enable the RabbitMQ broker for an existing profile, :ref:`install RabbitMQ <installation:guide-complete:rabbitmq>` and then run ``verdi profile configure-rabbitmq``.
By default, ``verdi presto`` creates a profile that can work without any server running in the background.
In particular:
- for the **database** (where the querable part of the data belonging to the AiiDA profile is stored): it uses SQLite instead of PostgreSQL.
- for the **broker** (that takes care of communication between processes): it does not use the RabbitMQ message broker.
For quick orientation, here is a matrix with the possible combinations of database and broker:
+----------------------+----------------------------------------------------+-------------------------------------------------------------+
| | **SQLite database** | **PostgreSQL database** |
+======================+====================================================+=============================================================+
| **No broker** | Quick start with AiiDA | [*not really relevant for any usecase*] |
+----------------------+----------------------------------------------------+-------------------------------------------------------------+
| **RabbitMQ broker** | Production (low-throughput; beta, has limitations) | Production (maximum performance, ideal for high-throughput) |
+----------------------+----------------------------------------------------+-------------------------------------------------------------+
The table below gives a quick overview of the functionality that is not supported in those cases:
No broker (no RabbitMQ)
-----------------------
No broker service needs to be installed and running on the system, however:
- Not suitable for high-throughput workloads (a poll-based mechanism is used rather than an event-based one)
- Cannot run the daemon (no `verdi daemon start/stop`), therefore processes cannot be submitted to the daemon (i.e., one can only use ``run()`` instead of ``submit()`` to execute calcuulations and workflows).
**NOTE**: Calculations can still be run on remote computers.
- Cannot play, pause, kill processes.
.. note::
Once you setup a profile with ``verdi presto``, you can at a later stage remove very simply the limitations of not having a broker.
This can be achieved by first installing the RabbitMQ broker (see :ref:`install RabbitMQ <installation:guide-complete:rabbitmq>`).
Then, just run ``verdi profile configure-rabbitmq``.
SQLite instead of PostgreSQL (as the database)
----------------------------------------------
No database service needs to be installed and running on the system, however:
- Not suitable for high-throughput workloads (writes from multiple processes to the database are serialized)
- No support for ``has_key`` and ``contains`` operators in query builder
- No support for ``QueryBuilder.get_creation_statistics``

Copy link
Member

@giovannipizzi giovannipizzi Jul 19, 2024

Choose a reason for hiding this comment

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

All the next points to be discussed:

  • I tried to make the 2x2 matrix only to explain easily what each combination might be useful for. I convert the rest to a bullet-point list.

  • I also added for both that it's not suitable for high-throughput, for different reasons.

  • I still added a 'beta' version to the RMQ+SQLite, as I think we are not 100% sure of all the missing functionalities I think. We can remove it after benchmarking. Or are we already ready to recommend it for low-throughput production jobs? I'm open to discussion of course.

Copy link
Contributor

@GeigerJ2 GeigerJ2 left a comment

Choose a reason for hiding this comment

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

Very nice, thanks @sphuber and @giovannipizzi! I just added two minor comments.

Also, should we mention that verdi presto can be used to set up a production profile if RMQ and PSQL are available and --use-postgres is used? Possibly as a note below the last tip, something like (would have added it at the relevant position, but GH doesn't allow adding suggestions on unchanged lines):

.. note::
If both, RabbitMQ and PostgreSQL are available on your system, it *is* in principle possible to create a *production profile* using `verdi presto`. As mentioned above, RabbitMQ will be detected automatically, and the flag `--use-postgres` can be used to enable PostgreSQL. This allows for less customization than the :ref:`complete installation <installation:guide-complete>`, but can serve as a shortcut to create a production profile.

This risks some possible confusion, as we generally mention that verdi presto isn't the typical route to a production profile, but I personally often use it in that way. Just a suggestion, don't have a strong opinion on it, we can also decide not to include it.

docs/source/installation/guide_quick.rst Outdated Show resolved Hide resolved
docs/source/installation/guide_quick.rst Outdated Show resolved Hide resolved
@sphuber sphuber force-pushed the fix/docs-improve-broker-storage-limitations branch 2 times, most recently from 3d192b7 to c19d792 Compare July 21, 2024 21:38
@sphuber
Copy link
Contributor Author

sphuber commented Jul 21, 2024

Thanks for the review @GeigerJ2

Also, should we mention that verdi presto can be used to set up a production profile if RMQ and PSQL are available and --use-postgres is used?

This was mentioned in a separate tip in the section on limitations of SQLite. As you noted in another comment, I mostly just added Giovanni's suggestions to my existing content, leading to some duplication. I have now rewritten the part to integrate the parts better. As to your suggestion for adding the note: there are two separate notes that show that verdi presto can be used to setup a profile using the services. I am not sure we need another one that combines them. Or at least, I would rather add that to the complete guide.

@GeigerJ2
Copy link
Contributor

As to your suggestion for adding the note: there are two separate notes that show that verdi presto can be used to setup a profile using the services. I am not sure we need another one that combines them. Or at least, I would rather add that to the complete guide.

That's fine for me, we can add that as a tip to the complete guide. On that note, I realized that the option --profile-name for verdi profile setup is still named --profile in the complete guide. Iirc, we made it --profile-name for both, that and verdi presto (that's also what --help is telling me). I'd add a commit to fix it (and add the note), but not sure if that's OK for you, so adding it as a comment here for now. Apart from that, looks all good to me!

@sphuber sphuber force-pushed the fix/docs-improve-broker-storage-limitations branch from c19d792 to a681fad Compare July 22, 2024 08:08
@sphuber
Copy link
Contributor Author

sphuber commented Jul 22, 2024

That's fine for me, we can add that as a tip to the complete guide. On that note, I realized that the option --profile-name for verdi profile setup is still named --profile in the complete guide. Iirc, we made it --profile-name for both, that and verdi presto (that's also what --help is telling me). I'd add a commit to fix it (and add the note), but not sure if that's OK for you, so adding it as a comment here for now. Apart from that, looks all good to me!

Thanks, since I don't want to squash these three commits, I have added the fix to the last commit. I have also added the tip to the complete guide. Should be good for final review.

Copy link
Contributor

@GeigerJ2 GeigerJ2 left a comment

Choose a reason for hiding this comment

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

Now, the tip of using --use-postgres is contained twice in the complete installation guide. While it's a bit redundant, I think it's fine. Being able to avoid all the PSQL commands, is an important message to drive home.

The only thing I'm missing, and this is my main point, is that with RMQ installed, it is possible to create a production profile using verdi presto, which is not mentioned explicitly. We note in multiple places that a verdi presto profile has all these limitations, but in that scenario, it's not really true. So I propose we add:

.. tip::
    The creation of the PostgreSQL user and database as explained below is implemented in an automated way in the ``verdi presto`` command.
    Instead of performing the steps below manually and running ``verdi profile setup core.psql_dos``, it is possible to run:
    .. code-block::
        verdi presto --use-postgres

Thus, if RabbitMQ is also installed correctly and automatically detected, all AiiDA functionality is available. Therefore, with both services being functional, ``verdi presto --use-postgres`` can be used as a quick way (shortcut?) to reach a production profile.

(I also removed the second "manually". GH, why don't you let me comment on unchanged lines...)

@sphuber
Copy link
Contributor Author

sphuber commented Jul 22, 2024

So I propose we add:

Do we add this to the beginning of the instructions for the database creation of PSQL?

🤦 I see that this already exists. I thought you were adding it, but you are just suggesting to modify it slightly.

(I also removed the second "manually". GH, why don't you let me comment on unchanged lines...)

what "manually" are you referring to here?

Never mind I see where now.

sphuber added 3 commits July 22, 2024 11:34
A table is added to the quick installation guide that gives a more
succinct overview of the limitations of a profile created without a
broker and with SQLite instead of PostgreSQL. This was already explained
in more detail in text, but that was not complete and could be too dense
for users, making them likely to skip it.

The code is updated to provide a link to this documentation section
whenever an error is displayed that a broker is not configured. This way
users can try to understand why the functionality they are trying to use
is not supported and how, if they really care about it, they can go
about still installing and configuring RabbitMQ after the fact.
This admonition is not that useful since it still refers the user to an
external website and the target information is not immediately obvious
either. Besides, if the installed Python version is not supported, this
will automatically be reported by the package manager.
The fact that the quick installation guide can lead to a profile that
has certain limitations should be the first thing to be read.
@sphuber sphuber force-pushed the fix/docs-improve-broker-storage-limitations branch from a681fad to 6299874 Compare July 22, 2024 09:43
@sphuber
Copy link
Contributor Author

sphuber commented Jul 22, 2024

ok @GeigerJ2 how about this. Sorry for being slow in understanding what you were suggesting.

@GeigerJ2
Copy link
Contributor

Yeah, I repeated part of the tip. As I wrote my comment, I was thinking if I should remove that part to avoid exactly the possible confusion that occurred ^^ Very annoying one cannot annotate/suggest on unchanged lines... 😅
Reads perfect now, thanks for implementing it, LGTM!

@sphuber sphuber merged commit d73731f into aiidateam:main Jul 22, 2024
12 checks passed
@sphuber sphuber deleted the fix/docs-improve-broker-storage-limitations branch July 22, 2024 11:17
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