Skip to content

IGNITE-28007 split TablePartitionResourcesFactory out of TableManager#7701

Open
dant3 wants to merge 3 commits intoapache:mainfrom
unisonteam:IGNITE-28007
Open

IGNITE-28007 split TablePartitionResourcesFactory out of TableManager#7701
dant3 wants to merge 3 commits intoapache:mainfrom
unisonteam:IGNITE-28007

Conversation

@dant3
Copy link
Contributor

@dant3 dant3 commented Mar 4, 2026

https://issues.apache.org/jira/browse/IGNITE-28007

Key change points:

  1. Removed fields that are now only needed by the factory (or only in the constructor)
  2. Added the factory for partition-related components - it only takes care about pure construction and dependency injection without mixing it with the lifecycle management which TableManager did and still does.
  3. Refactored some methods that mixed components creation and lifecycle:
    • preparePartitionResourcesAndLoadToZoneReplicaBusy - split creation and lifecycle concerns
    • registerPartitionTableStatsMetrics — now registers pendingWriteIntentsSuppliers (was previously done inside createPartitionUpdateHandlers). The upstream also changed the metric from register-only to register-and-enable.
    • unregisterPartitionMetrics — extracted as a separate method from the inline code in the table-drop path. Takes (TablePartitionId, String tableName) instead of accessing table.name() directly.


@Override
public String toString() {
return S.toString(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure we need this?


fullStateTransferIndexChooser = new FullStateTransferIndexChooser(catalogService, lowWatermark, indexMetaStorage);

partitionResourcesFactory = new TablePartitionResourcesFactory(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create this factory in IgniteImpl and just insert it as a dependency into TableManager?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't because this factory depends on some things that are created inside of the TableManager currently. This also makes the surface of the change way bigger than ideal.

MvGc mvGc,
FullStateTransferIndexChooser fullStateTransferIndexChooser
) {
this.txManager = requireNonNull(txManager, "txManager");
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't be bothered with checks tbf. We have annotations for that purpose.

catalogService,
table.schemaView(),
indexMetaStorage,
topologyService.localMember().id(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nitpick, we can cache id.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants