internal/plugin+internal/plugin6: Fix provider schema caching

[bflad]

Reference: #33486

The intention of this logic is to use the global provider schema cache if it is available for a given provider address based on the GetProviderSchemaOptional server capability. Previously the logic was errantly checking the named return early, which was not populated yet with the server response. This flips the logic to properly use the server response to determine whether the cache should be set, thereby still gating the caching based on the server capability.

In a configuration with 19 hashicorp/aws provider instances and an associated data source, this drops the received GetProviderSchema calls from 39 calls to 1 call with this change. Maximum RSS dropped from 1.22 GB to 189 MB.

Target Release

1.6.x

Draft CHANGELOG entry

N/A (already changelog'd as #33486)

[bflad]

While this does actually enable cache usage, in my negative, real-world testing of this with hashicorp/aws version 5.14.0 and the same configuration, there is a significant performance decrease with AttachSchemaTransformer execution compared to Terraform 1.6.0-alpha20230816 (we're talking 30+ seconds each graph walk).

[jbardin]

The reason this slows everything down is that it reverses the logic required by core to actually use the cache. The cache must be set for every single provider, because otherwise core needs to spin up a new provider instance each time it wants to fetch the schema, which is why you run into performance issues. Because every provider method internally must call GetProviderSchema to both get the schema for serialization and ensure that GetProviderSchema was called for provider instances which require it, the condition must be checked at the start of GetProviderSchema, not the end.

Since the aws provider does not yet enable GetProviderSchemaOptional, we cannot get the benefits yet of the schema caching on the provider side because we will always need to spin up every provider to call GetProviderSchema again. I'm actually not able to replicate your findings though, with applying this patch resulting in more calls to GetProviderSchema overall, nor can I explain the drop in RSS since the total number of provider processes should be the same.

[bflad]

Superseded by #33807 👍

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.