[slachiewicz]

What changes were proposed in this pull request?

Adds Thrift's contrib/fb303/if/fb303.thrift as standalone-metastore/metastore-common/include/share/fb303/if/fb303.thrift. The metastore-common Thrift execution already passes -I ${basedir}/include, so include "share/fb303/if/fb303.thrift" resolves there with no change to the IDL or the POM.

Why are the changes needed?

The include currently resolves only through -I ${thrift.home}, which requires fb303 to have been installed separately into the Thrift prefix. Thrift no longer installs it and declined to (THRIFT-6389), so regenerating the metastore bindings needs a manual download first. Dropping FacebookService itself is HIVE-18249.

Does this PR introduce any user-facing change?

No. The file is identical in Thrift 0.16.0 (the pinned libthrift.version), 0.24.0 and master.

How was this patch tested?

Verified: thrift -strict --gen java:beans,generated_annotations=undated --gen cpp --gen php --gen py --gen rb on hive_metastore.thrift → 1327 files, diff -r-identical to the same run resolving fb303 through a thrift.home-style prefix; without the file it fails with Could not find include file. Run with Thrift 0.24.0; I have not run the thriftif Maven profile with 0.16.0.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[dengzhhu653]

nit: can we change the namespace to org.apache.hive.com.facebook.fb303 ? per my understanding, it should be compatible with old clients.

[slachiewicz]

I'd keep com.facebook.fb303 here. The wire format wouldn't change, but the namespace becomes the Java supertype of ThriftHiveMetastore.Iface (@Public @Stable) and the return type of getStatus(), so existing implementers of Iface would stop compiling. This PR only makes the IDL resolvable without changing any generated output.

[dengzhhu653]

can we remove the libfb303 dependency from root pom and the standalone-metastore pom?

[slachiewicz]

libfb303 still has to supply the Java classes: the generated ThriftHiveMetastore extends com.facebook.fb303.FacebookService, and BaseHandler extends FacebookBase, which is hand-written in that jar. Renaming the package is wire-compatible, but it changes the supertype of ThriftHiveMetastore.Iface (@Public @Stable) and the return type of getStatus(), so implementers such as waggle-dance's FederatedHMSHandler and metacat's CatalogThriftHiveMetastore would stop compiling. I'd rather keep this PR output-neutral and drop libfb303 together with FacebookService in HIVE-18249 for a major release. Happy to do that follow-up.

[InvisibleProgrammer]

It is only half way of the work. I faced with the same working on the thrift upgrade and made the same conclusion at my thrift upgrade PR here: #6653

Let me draw your attention on this commit specifically: 8aadf78
It is not only adds the thrift file but also removes the existing dependencies as well.

And I think this change is required too: https://github.com/apache/hive/pull/6653/changes#diff-deb369624f92d4833d1c243ccd5f97cac71a281cf39b7326bda9a1013fac728aL25

[slachiewicz]

Closing as superseded. #6893 added fb303.thrift under src/main/thrift and switched the include to include "fb303.thrift", so master no longer needs a separately installed fb303. Removing the libfb303 dependency is covered by #6938 (HIVE-30237).