Skip to content

[IOTDB-5674] Multi-Mode IMNode Management#9243

Merged
MarcosZyk merged 30 commits intoapache:masterfrom
Cpaulyz:clearSchemaFile
Mar 15, 2023
Merged

[IOTDB-5674] Multi-Mode IMNode Management#9243
MarcosZyk merged 30 commits intoapache:masterfrom
Cpaulyz:clearSchemaFile

Conversation

@Cpaulyz
Copy link
Contributor

@Cpaulyz Cpaulyz commented Mar 8, 2023

Copy link
Member

@SzyWilliam SzyWilliam left a comment

Choose a reason for hiding this comment

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

Bravo work!

IMeasurementSchema getSchema();

TSDataType getDataType(String measurementId);
void setSchema(IMeasurementSchema schema);
Copy link
Contributor

Choose a reason for hiding this comment

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

We assume the schema is immutable. Is this interface necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

use to implement moveDataToNewMNode()

Copy link
Contributor

Choose a reason for hiding this comment

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

There seems only device node replacement. There won't be any measurement node replacement. Maybe we can push down the moveDataToNewMNode() to only InternalMNode and its subclass.

Comment on lines +25 to +28
import org.apache.iotdb.commons.schema.node.info.IMeasurementInfo;
import org.apache.iotdb.commons.schema.node.role.IDatabaseMNode;
import org.apache.iotdb.commons.schema.node.role.IDeviceMNode;
import org.apache.iotdb.commons.schema.node.role.IMeasurementMNode;
Copy link
Contributor

Choose a reason for hiding this comment

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

The device and measurement related code seems only being used in server package.

Comment on lines -84 to +88
void pin(IMNode node) throws MetadataException;
void pin(N node) throws MetadataException;

void unPin(IMNode node);
void unPin(N node);

void unPinPath(IMNode node);
void unPinPath(N node);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe these interfaces should only be behavior of CachdMTreeStore. The common behavior are releaseNode and releasePath, which only used in Traverser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.. but it's still be used in SchemaRegionSchemaFileImpl. I added these todo's about a month ago..

  // TODO: This interface should not be exposed to SchemaRegion
  public void pinMNode(ICacheMNode node) throws MetadataException {
    store.pin(node);
  }

  // TODO: This interface should not be exposed to SchemaRegion
  public void unPinMNode(ICacheMNode node) {
    store.unPin(node);
  }

  private void unPinPath(ICacheMNode node) {
    store.unPinPath(node);
  }

  // TODO: This interface should not be exposed to SchemaRegion
  public void updateMNode(ICacheMNode node) throws MetadataException {
    store.updateMNode(node);
  }

Copy link
Contributor

Choose a reason for hiding this comment

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

At least the Traverser shall not sense these interface? It takes a big effort to refactor the implementations in SchemaRegionSchemaFileImpl.

Comment on lines 62 to +64
// measurement in template should be processed only if templateMap is not null
protected Map<Integer, Template> templateMap;
protected IMNodeFactory<N> nodeFactory;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be pushed down to MeasurementTraverser. This class will act as a common traverser and will be placed in package node-commons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's ok to put all the getChild logic in a base class. It will also make it easier to read.

@Cpaulyz Cpaulyz changed the title [IOTDB-5674] Multi-Model IMNode Management [IOTDB-5674] Multi-Mode IMNode Management Mar 10, 2023
@MarcosZyk MarcosZyk merged commit 78b2b27 into apache:master Mar 15, 2023
@Cpaulyz Cpaulyz deleted the clearSchemaFile branch March 15, 2023 14:14
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.

3 participants