Skip to content

Core: Remove ThreadPools.WORKER_THREAD_POOL_SIZE_PROP update warning log message - #17812

Merged
huaxingao merged 2 commits into
apache:mainfrom
dramaticlly:core-1.12-marker-cleanup
Sep 1, 2026
Merged

Core: Remove ThreadPools.WORKER_THREAD_POOL_SIZE_PROP update warning log message#17812
huaxingao merged 2 commits into
apache:mainfrom
dramaticlly:core-1.12-marker-cleanup

Conversation

@dramaticlly

Copy link
Copy Markdown
Contributor

2 changes

  1. Remove ThreadPools.WORKER_THREAD_POOL_SIZE_PROP. The field was deprecated in Core: Support set system level properties with environmental variables #5659

  2. Update the ManifestReader warning log for reading specById fallback from 1.12.0 to 2.0.0.
    fallback cannot be honored in 1.12.0 because

    ManifestFiles.readDeleteManifest(manifest, fileIO, null)) {
    .

…ifest spec fallback removal version
Resolves the two remaining deprecation markers in iceberg-core that
promised action "in 1.12.0" but were missed by the removal PRs.
Remove ThreadPools.WORKER_THREAD_POOL_SIZE_PROP. The field was
deprecated in apache#5659 in favor of SystemConfigs.WORKER_THREAD_POOL_SIZE
and retargeted from 2.0.0 to 1.12.0 by apache#14392. It had no callers
anywhere in the repository outside its own declaration and no
references in the docs, and it was the only survivor of apache#14392's
13-file batch after apache#17700 removed the rest. The corresponding
java.field.removed break is recorded in .palantir/revapi.yml under the
1.11.0 baseline.
Correct the ManifestReader warning for the partition-spec-from-file-
metadata fallback to say 2.0.0 rather than 1.12.0. The fallback cannot
be honored in 1.12.0 because its only production caller is
BaseSnapshot:276, which passes a null spec map while serving
Snapshot.addedDeleteFiles(FileIO) and removedDeleteFiles(FileIO).
Those accessors are themselves already marked "@deprecated will be
removed in 2.0.0; use SnapshotChanges#builderFor(Table) instead", and
because they receive only a FileIO they structurally cannot supply a
spec map. The fallback must therefore outlive 1.12.0, and 2.0.0 aligns
its removal with that of its last caller.
LOG.warn(
"Reading partition spec from manifest file metadata is deprecated and will be "
+ "removed in the 1.12.0 release. Pass specsById to avoid reading from file metadata: {}",
+ "removed in the 2.0.0 release. Pass specsById to avoid reading from file metadata: {}",

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@RussellSpitzer I updated the warning log message to 2.0 as today

ManifestFiles.readDeleteManifest(manifest, fileIO, null)) {
still pass a null specById. Given class of baseSnapshot does not have a way to access specById directly from table metadata, it will continue to use the fallback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not directly related to prop removal i think, when merging this lets make sure we add the commit description accordingly !

@dramaticlly

Copy link
Copy Markdown
ContributorAuthor

@huaxingao if you can help take a look at this one as well

@huaxingaohuaxingao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@uros-buros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, LGTM

@dramaticlly

Copy link
Copy Markdown
ContributorAuthor

thanks @huaxingao and @uros-b for the review, shall we merge it if it's ready?

@dramaticlly

Copy link
Copy Markdown
ContributorAuthor

I think this is the last piece needed to close #16445

Comment thread.palantir/revapi.yml Outdated
@huaxingao
huaxingao merged commit 6164440 into apache:mainSep 1, 2026
40 checks passed
@huaxingao

Copy link
Copy Markdown
Contributor

Thanks @dramaticlly for the PR! Thanks everyone for the review!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@dramaticlly@huaxingao@nssalian@singhpk234@uros-b