Skip to content

Conversation

@fsk119
Copy link
Member

@fsk119 fsk119 commented Sep 22, 2022

…ferent config

What is the purpose of the change

The connection may leak when users have different config. Here, we fix this like hive does to use the same config to create the HiveMetaStoreClient.

@flinkbot
Copy link
Collaborator

flinkbot commented Sep 22, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Contributor

@yuzelin yuzelin left a comment

Choose a reason for hiding this comment

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

LGTM. I left several comments, please check out.

// Trigger the creation of the HiveMetaStoreClient to use the same HiveConf. If the
// initial HiveConf is different, it will trigger the PersistenceManagerFactory to close
// all the alive PersistenceManager in the ObjectStore, which may get error like
// "PersistenceManager is closed" in the later connection.
Copy link
Contributor

Choose a reason for hiding this comment

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

The exact exception message is “Persistence Manager has been closed”

.setDefaultCatalog(catalogName)
.addSessionConfig(sessionConfig)
.build());
// set variables to HiveConf or Session's conf
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this comment line (329) should lay before line 315 0r 319.

@fsk119
Copy link
Member Author

fsk119 commented Sep 23, 2022

Thanks for your review. Because the comment is not releated to the code, I will this offline.

@fsk119
Copy link
Member Author

fsk119 commented Sep 23, 2022

The failed test is not releated to the fix. Merging...

@fsk119 fsk119 changed the title [FLINK-29229][hive] Fix ObjectStore leak when different users has dif… [FLINK-29274][hive] Fix ObjectStore leak when different users has dif… Sep 23, 2022
fsk119 added a commit that referenced this pull request Sep 23, 2022
@fsk119 fsk119 closed this Sep 23, 2022
fsk119 added a commit to fsk119/flink that referenced this pull request Sep 23, 2022
fsk119 added a commit to fsk119/flink that referenced this pull request Sep 26, 2022
@fsk119 fsk119 deleted the fix-pm branch March 8, 2023 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants