Ensure ImportJobTest is not flaky by checking WriteToStore metric and requesting adequate resources for testing

[davidheryanto]

feast.ingestion.ImportJobTest sometimes fail unpredictably because not all the keys ingested in Redis are found during retrieval, but the test log does not provide useful info for debugging the failure.

Example of such error:

runPipeline_ShouldWriteToRedisCorrectlyGivenValidSpecAndFeatureRow

  java.lang.AssertionError: 
  Key not found in Redis: ...

This pull request:

1. Adds a check whether the ingestion job has finished writing elements by checking the metric for no of elements written to the store periodically. Only when this counter no longer changes, the correctness of the ingested FeatureRow is evaluated. Previously the check is based on a fixed amount of duration (which is less reliable due to the the variability in the resources and input samples in the running environment)

The reason for using metric utility for this check is because the pipeline is a streaming pipeline with
unbounded source. Hence, the pipeline is always running or failed, i.e. no completed succesfully
state to check when it has ingested all the Feature Row.

Beam built in metric utility is used here (so it has no external dependencies on external metric collectors, such as statsd or prometheus) and this metric counts the no elements
WriteToStore transform has processed.
https://beam.apache.org/documentation/programming-guide/#metrics

2. Adds resources request for the Pod running the test. Previously, the resources that the test Pod get assigned depends on how overloaded the test cluster is. By having a guaranteed amount of CPU and memory, the duration of tests should be more predictable.

3. Adds more debugging info when such test fails:

• Printing the output of redis INFO. This is useful for e.g. to check the count of keys ingested. If the count is fewer than expected, probably, we need to let ingestion job runs a bit longer because it hasn't fully ingested the complete sample input data.
• Printing one random sample of element in Redis, to check that at least some FeatureRow is correctly ingested.

These should ensure ImportJobTest test result is reproducible.

[feast-ci-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidheryanto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [davidheryanto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[woop]

Thanks for this @davidheryanto

Should we perhaps rename the PR to be more descriptive? Its not clear that this ensures any less flakyness.

[davidheryanto]

/hold
Need to update PR title and consider tracking the ingestion progress.

[davidheryanto]

/hold cancel
Updated PR title and add check for WriteToStore metrics

[zhilingc]

/lgtm

[zhilingc]

/retest

[feast-ci-bot]

@davidheryanto: Updated the config configmap in namespace default using the following files:

• key config.yaml using file .prow/config.yaml

In response to this:

feast.ingestion.ImportJobTest sometimes fail unpredictably because not all the keys ingested in Redis are found during retrieval, but the test log does not provide useful info for debugging the failure.

Example of such error:

runPipeline_ShouldWriteToRedisCorrectlyGivenValidSpecAndFeatureRow

 java.lang.AssertionError: 
 Key not found in Redis: ...

This pull request:

1. Adds a check whether the ingestion job has finished writing elements by checking the metric for no of elements written to the store periodically. Only when this counter no longer changes, the correctness of the ingested FeatureRow is evaluated. Previously the check is based on a fixed amount of duration (which is less reliable due to the the variability in the resources and input samples in the running environment)

The reason for using metric utility for this check is because the pipeline is a streaming pipeline with
unbounded source. Hence, the pipeline is always running or failed, i.e. no completed succesfully
state to check when it has ingested all the Feature Row.

Beam built in metric utility is used here (so it has no external dependencies on external metric collectors, such as statsd or prometheus) and this metric counts the no elements
WriteToStore transform has processed.
https://beam.apache.org/documentation/programming-guide/#metrics

2. Adds resources request for the Pod running the test. Previously, the resources that the test Pod get assigned depends on how overloaded the test cluster is. By having a guaranteed amount of CPU and memory, the duration of tests should be more predictable.

3. Adds more debugging info when such test fails:

• Printing the output of redis INFO. This is useful for e.g. to check the count of keys ingested. If the count is fewer than expected, probably, we need to let ingestion job runs a bit longer because it hasn't fully ingested the complete sample input data.
• Printing one random sample of element in Redis, to check that at least some FeatureRow is correctly ingested.

These should ensure ImportJobTest test result is reproducible.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.