TIMX 494 - TIMDEXRunManager for producing ETL run metadata#143
Conversation
Why these changes are being introduced: One of the challenges the architecture of the TIMDEX parquet dataset presents is quick and easy metadata about ETL "runs" in the dataset. The year/month/day partitioning structure is very efficient for accessing a run if you know the date, where only a few parquet files are scanned, but it's not geared towards quickly isolating runs (parquet files) associated with a given source. Having metadata about runs provides a map to efficiently access meaningful subsets of data. One example would be fully refreshing a source in Opensearch. To do, you'd want to access all runs for a given source since, and including, the last run_type=full run. Those runs represent the current state of the source in TIMDEX. Unfortunately, this is not terribly efficient to naively perform with pyarrow or DuckDB, where potentially thousands of parquet files are touched. Similar to how Apache Iceberg (a parquet dataset architecture) works, we need some metadata about each "run" in the dataset which correlates to parquet file(s). How this addresses that need: A new class TIMDEXRunManager exists to provide this functionality. This class will produce a pandas dataframe of metadata about all runs in the dataset, including the explicit parquet filepath the run is associated with, in a highly efficient and parallelized way. The is achieved by: 1. Getting a list of all parquet files from the dataset. 2. Reading the *first* row from each file, which contains metadata about the run that produced the file. 3. Aggregating the results and grouping by "run_id". The result is a dataframe that provides a precise map of run metadata to parquet files in the dataset. With those parquet files identified, this unblocks further functionality for this library like "replaying" the runs for a given source in chronological order to refresh it in Opensearch. Side effects of this change: * None. No changes are made to pre-existing functionality, just the addition of this new information gathering class. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-493 * https://mitlibraries.atlassian.net/browse/TIMX-494
| - a high number is generally safe given the lightweight nature of the | ||
| thread's work, just reading a few parquet file header bytes | ||
| """ | ||
| with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: |
There was a problem hiding this comment.
Maybe not obvious on a first pass, but it is here that we are reading the metadata from parquet files in parallel. max_workers is set to 250, which is quite high, but believed to be safe given the lightweight nature of the HTTP request and response (just a few bytes).
|
|
||
| # group by run_id | ||
| grouped_runs_df = ( | ||
| ungrouped_runs_df.groupby("run_id") |
There was a problem hiding this comment.
Because a single ETL run may span multiple parquet files (we limit the parquet files to 100k rows), we must group by run_id to ensure that our final result is grouped at the ETL run level.
ehanson8
left a comment
There was a problem hiding this comment.
Great context for the new functionality, a few comments!
| - a high number is generally safe given the lightweight nature of the | ||
| thread's work, just reading a few parquet file header bytes | ||
| """ | ||
| with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: |
| ) | ||
| return grouped_runs_df | ||
|
|
||
| def get_current_source_parquet_files(self, source: str) -> list[str]: |
There was a problem hiding this comment.
Maybe this will change later as more methods are added but it feels like the method order should flip given that each method is contained in the one below it? There are certainly exceptions to every rule, but I thought we were generally leading with the highest-level of abstraction and then descending into more specific methods. This also may be a good DataEng meeting topic to discuss in more detail to clarify our norms!
There was a problem hiding this comment.
I think I agree! Thanks for pushing on this. As I develop a class like this, I'm often writing the lower level / private methods first to build up to the higher-level / public / interface methods.
I'll reorder, and actually privatize a couple.
I'm happy to discuss, my proposal would be:
- properties
- "magic" methods that override the
__X__dunder methods - public methods, the primary interface
- private methods, starting with leading underscores
_X
Internal ordering of 3, the public methods, I'd vote for dealer's choice, whatever makes sense.
Bringing it back here, if I were to make the truly private methods private, then the ordering kind of takes care of itself via that approach!
Thanks again though, good suggestion.
There was a problem hiding this comment.
This is so much more logical and readable to me, great change! I agree with your proposed order, @jonavellecuerdo should also weigh in. We should use private methods more frequently to achieve this type of order (I still forget to at times as well) but the behavior of this class is so much clearer with the public methods up top
|
Also, it seems the branch protection rules should be updated to block merging without an approval |
Good call. Based on new github permissions, I think we'll need to ask an admin. |
Pull Request Test Coverage Report for Build 15163424367Details
💛 - Coveralls |
ehanson8
left a comment
There was a problem hiding this comment.
Excellent changes! Consider this approved from me but letting @jonavellecuerdo handle the final approval since she self-assigned
| ) | ||
| return grouped_runs_df | ||
|
|
||
| def get_current_source_parquet_files(self, source: str) -> list[str]: |
There was a problem hiding this comment.
This is so much more logical and readable to me, great change! I agree with your proposed order, @jonavellecuerdo should also weigh in. We should use private methods more frequently to achieve this type of order (I still forget to at times as well) but the behavior of this class is so much clearer with the public methods up top
Ah, we should also ask if default rules can be created for new repos in this new permissions scheme so we don't have to bother an admin each time |
jonavellecuerdo
left a comment
There was a problem hiding this comment.
Really cool stuff in this PR. Approved! 😃
Purpose and background context
This PR adds a new class
TIMDEXRunManagerthat is a utility class for efficiently gathering metadata about all runs in the TIMDEX parquet dataset.The original requirements of this library were to support efficient writing and reading from the dataset over the course of a single ETL run. For example Transmogrifier writes to the dataset where all records have a shared
run_idand TIM reads from the dataset for thatrun_idand indexes those records to Opensearch. This was made very efficient by the year/month/day partitioning strategy, where TIM instantly narrows it down to parquet files under those partitions and then just reads the metadata from those few parquet files to get the file(s) that match arun_id.However, it was assumed the dataset and this library would be capable of far more over time! One known use case: providing easy access to all the records for a given TIMDEX source that are currently in Opensearch. Let's think of this as the "current records for a source." To provide this functionality, we first need some understanding of the "runs" in the dataset specific to that source, including the
run_date,run_type,run_id, associated parquet file, etc.While technically we could load the whole dataset and use pyarrow dataset filtering or DuckDB, this is quite inefficient. This new class relies on some things we -- the dataset creators -- know about the dataset to support highly-parallelized run metadata retrieval:
run_idto create a full picture of a run from this collected metadataAnd this is precisely how this utility class approaches producing dataset runs metadata:
TIMDEXDatasetwhich has a list of all parquet files in the datasetrun_idThis has proven to be very fast, around 1-2 seconds given even hundreds of parquet files. With that final run metadata in hand, it paves the way for future functionality like yielding only the currrent version of a record, for a source.
How can a reviewer manually see the effects of these changes?
1- For all tests below, set AWS Dev Admin credentials (using a test bucket with a static clone of prod data)
2- Start an ipython shell:
Sometimes an alternate approach can illuminate the proposed one. Let's use DuckDB to get ETL run metadata.
Define a function that uses a SQL query to get at run data, grouping by
run_id:Now, let's run this for a few different sources:
The first run for
libguidesand evendspacelooked promising: 10-12 seconds and low memory usage. But what happened withalma?Unfortunately, DuckDB doesn't know some things about our dataset that it could leverage. It's amazing that it's this fast, but for
almawe can see it's very briefly touching each row in the dataset to get that run ETL metadata, which is not needed. Given thatalmais by far the largest, getting ETL run data for all sources would be about the same time (just a little slower).We would see similar performance results using
pyarrow.dataset.filter()andpyarrow.dataset.group_by().Though we want run-level metadata, we also know that for each parquet file its associated ETL run metadata is present in every row, e.g. the first, single row. It's no shade on DuckDB or
pyarrow, they simply don't know as much about dataset and thus touch much more data than needed.This PR body probably isn't the best place for in-depth analysis of those approaches, but suffice to say, tried many angles and just couldn't get close to matching the performance of the custom class demonstrated next.
Now, let's use the
TIMDEXRunManagerto get the same data.Configure the dev logger for more output:
Next, load a normal
TIMDEXDatasetinstance with represents the full dataset:Note the quick load time, unchanged from before, discovering all parquet files in the dataset.
Observe how many parquet files comprise the dataset:
Now, load a
TIMDEXRunManagerinstance and retrieve ETL run data for all runs:Metadata about all ~270 ETL runs comes back in about 1-3 seconds! A fraction of a single, small source using DuckDB, and orders and orders magnitude faster than touching sources like
alma. Furthermore, this will continue to scale as the number of parquet files grows.Lastly, let's get an ordered list of all parquet files for a given source that represent the "current" records in TIMDEX:
Note it's basically instant, given that we've cached the full dataframe of ETL run metadata. If we had not done that step yet, this would take that 1-3 seconds we saw above.
A valid question would now be, "what do we do with a list of reverse chronologically ordered parquet files?" The answer is we use that list to load a new
TIMDEXDatasetinstance with those specific, ordered parquet files.This use of the valuable list of ordered, specific parquet files leads into a future PR which will utilize this valuable information to provide an efficient way to yield only "current" records for a given TIMDEX source.
Includes new or updated dependencies?
NO
Changes expectations for external applications?
NO
What are the relevant tickets?
Developer
Code Reviewer(s)