microsoft / microsoft/microsoft-performance-toolkit-sdk

When using the Engine, it's possible to enable a table that will not have its data available

Open
#195 0 comments 0 reactions 1 assignee View on GitHub

@mslukebo is already working on this.

Since Mar 3, 2022.

bug engine untriaged
Dominant language
C#
Stars
239
Forks
78
Avg merge
4d 3h
Merged PRs (30d)
2

Description

Describe the bug
When you use the Engine to process file(s) with multiple plugins concurrently loaded, it's possible to successfully enable a table that cannot be built due to its cookers never receiving data.

For example, consider the case where two plugins are loaded: PluginCSV and PluginTSV. PluginCSV and PluginTSV can process CSV and TSV files, respectively. Now, consider the data source set only contains a CSV file. PluginTSV will not create a CustomDataProcessor or SourceParser since it was not asked to process any files. This means source cookers hooked up to PluginTSV's SourceParser will never receive data, and tables that rely on any cookers connected to PluginTSV's parser will not be able to build.

In this situation, a call to Engine.EnableTable will succeed. This is unintuitive, since the successful call implies that the table can be built.

To Reproduce
Steps to reproduce the behavior:

  1. Create plugins that can process disjoint data sources
  2. Concurrently load these plugins into a PluginSet
  3. Create a DataSourceSet for this PluginSet that contains only data sources for a subset of the plugins
  4. Create an Engine with this DataSourceSet
  5. Call TryEnableTable for any table/cooker associated with a plugin that does not have a datasource in the DataSourceSet.

Expected behavior
The TryEnableTable fails and returns false

Actual behavior
TheTryEnableTable succeeds and returns true.

Version information (please complete the following information):

  • SDK version: 1.0.16
  • Engine version (if applicable): 1.0.16
  • WPA version (if applicable): N/A

Additional context
The following test, which can be added to ToolkitEngineTests, demonstrates the expected behavior and currently fails.

  [TestMethod]
  [IntegrationTest]
  public void TryEnableTable_NoDataSources_Fails()
  {
      this.DefaultSet.AddDataSource(new FileDataSource("test" + Source123DataSource.Extension));
      using var sut = Engine.Create(new EngineCreateInfo(this.DefaultSet.AsReadOnly()));

      var cooker = Source4DataCooker.DataCookerPath;

      Assert.IsFalse(sut.TryEnableTable(Source4DataCookerTable.TableDescriptor));
  }

This is NOT an issue with TryEnableCooker. EnableCooker's documentation states:

/// <exception cref="NoDataSourceException">
///     There are inadequate data sources in <see cref="DataSourcesToProcess"/>
///     in order for the specified cooker to participate in processing.
/// </exception>

EnableTable, however, does not document throwing NoDataSourceException. Since we are able to throw this exception for cookers, we should be able to throw it for tables as well.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.