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

Filesystem monitor database fix #1216

Merged

Conversation

bhilbert4
Copy link
Collaborator

This PR makes a few tweaks to the filesystem monitor.

Corrected sorting of log entries so that the most recent are always shown at the top.
Fixed bug that was preventing the plot of number of files vs filetype from appearing.
Made the database insert for the filesystem monitor robust against new file suffixes that are not in the enum suffixes of the table definition. (This was preventing the monitor from completing, as it tried to add an entry for 'msa' files.)
Tweaked the information stored for the central store and server directories.
Added plots for the central store and server directories, so that we can monitor disk space usage.

@bhilbert4
Copy link
Collaborator Author

This is ready for testing on one of the servers. The filesystem monitor will have to be run, and then we can bring up the dashboard and look at the results.

@mfixstsci
Copy link
Collaborator

@bhilbert4 With the addition of msa to the filetype enum in the filesystem db table, can the monitor be kicked off on one of the servers?

@bhilbert4
Copy link
Collaborator Author

I've been running it on the test server over the last few days. I put a block in the code to ignore the error from "msa" missing from the enum options. I still have a few tweaks to make. I need to turn off the plots that are made and saved when the monitor is run. I'm also toying with the idea of limiting how frequently the monitor walks through the mast filesystem and calculates the number of files and file sizes. That bit currently takes about an hour and 40 minutes on the test server, which seems pretty computationally expensive just to produce a pretty plot. So I think I want to cut down how frequently that runs, but I want to make sure that the table of log files is kept as up to date as possible, since that is pretty useful for us.

@mfixstsci
Copy link
Collaborator

@bhilbert4 great thank you for the response. Let me know if you need anything else with the database.

@bhilbert4 bhilbert4 changed the title [WIP]: Filesystem monitor database fix Filesystem monitor database fix Apr 4, 2023
@bhilbert4
Copy link
Collaborator Author

@mfixstsci this is finally ready for review. It's up on the test server at the moment. I've confirmed that the strange-looking data volume plots are accurately showing what is in the database, so it's not an issue with the plot generation.

@bhilbert4
Copy link
Collaborator Author

Hmm. The sqlalchemy commands in here may need to be updated though, depending on whether this is merged before or after #1234

@melanieclarke
Copy link
Collaborator

Hmm. The sqlalchemy commands in here may need to be updated though, depending on whether this is merged before or after #1234

@bhilbert4 - the newer syntax should be available in the older sqlalchemy version currently installed, so you can go ahead and update to the newer style before the other PR is merged if you want. Otherwise, I can take care of it later.

For the table updates, wherever execute is called, the pattern you want to follow is this:

        with engine.begin() as connection:
            connection.execute(FilesystemCharacteristics.__table__.insert(), new_record)

@bhilbert4
Copy link
Collaborator Author

Thanks @melanieclarke! I see you made the updates for the filesystem monitor in your PR. I did a bit of reorganizing in this PR, so I also made the changes in here. The lines themselves are identical to yours, so hopefully there won't be any conflicts.

@bhilbert4
Copy link
Collaborator Author

@mfixstsci this is ready for review. When tried on the test server, everything works well. The plots of data volume for some of the directories look strange, but I've confirmed that this is due to the cron job on the ops and test servers both running the filesystem monitor, and both placing their results in the ops database table. So it seems like we'll need to do some cleanup in that table. But in terms of the code, I think all is good.

Copy link
Collaborator

@mfixstsci mfixstsci left a comment

Choose a reason for hiding this comment

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

Hey @bhilbert4 all of the code changes look good to me. The test failure is sneaky, it was hiding in the __init__.py here: https://github.com/bhilbert4/jwql/blob/filesystem-monitor-database-fix/jwql/website/apps/jwql/monitor_pages/__init__.py#L2

@bhilbert4
Copy link
Collaborator Author

RTD build is failing due to trying to install codecov. This should be fixed once #1245 is merged.

@bhilbert4
Copy link
Collaborator Author

@mfixstsci tests are now all passing!

@mfixstsci
Copy link
Collaborator

@mfixstsci tests are now all passing!

WOOOOOO! I will review the changes and merge them!

Copy link
Collaborator

@mfixstsci mfixstsci left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for making that change @bhilbert4

@mfixstsci mfixstsci merged commit a7c71b5 into spacetelescope:develop Apr 17, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants