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

Measuring throughput of connectors #2605

Merged
merged 8 commits into from
Mar 25, 2021
Merged

Conversation

ChristopheDuong
Copy link
Contributor

What

Closes #2524

How

Add attempts metadata when tracking job completions

Recommended reading order

  1. airbyte-scheduler/src/main/java/io/airbyte/scheduler/JobTracker.java
  2. the rest

Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

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

lgtm.

worth noting that fetching the job output struck me that it could be dangerous because i didn't know what order saving the job output and doing the track calls happened. it looks like we do save job output before calling track so it should be okay.

if (lastAttempt.getOutput() != null && lastAttempt.getOutput().isPresent()) {
final JobOutput jobOutput = lastAttempt.getOutput().get();
final StandardSyncSummary syncSummary = jobOutput.getSync().getStandardSyncSummary();
metadata.put("duration", syncSummary.getEndTime() - syncSummary.getStartTime());
Copy link
Contributor

Choose a reason for hiding this comment

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

just want to sanity check that sync summary metrics are in seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's in milliseconds based on what's being saved in the syncSummary:

long startTime = System.currentTimeMillis();

Copy link
Contributor

Choose a reason for hiding this comment

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

if you look at the excel spreadsheet that john put together for the tracking plan he is expecting seconds. either his expectation needs to change to ms or we need to convert here. i have no preference, just want to make sure that the two of you are speaking the same language.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok will switch to seconds, thanks!

@ChristopheDuong ChristopheDuong merged commit 65bf03b into master Mar 25, 2021
@ChristopheDuong ChristopheDuong deleted the chris/track-throughput branch March 25, 2021 10:02
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.

Measuring throughput of connectors
2 participants