You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/07/13 11:46:08 UTC
[GitHub] [shardingsphere] SirMin opened a new issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
SirMin opened a new issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] SirMin closed issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
SirMin closed issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kimmking commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
kimmking commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-657604965
HintManager is designed to pass value from custom-hint codes to sharding engine, so in every query, it's a temp and once in user-side.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-657937431
Hi @SirMin sorry, do you mean the following one?
```java
public final class HintManager implements AutoCloseable {
**private static final ThreadLocal<HintManager> HINT_MANAGER_HOLDER = new ThreadLocal<>();**
private final Multimap<String, Comparable<?>> databaseShardingValues = HashMultimap.create();
private final Multimap<String, Comparable<?>> tableShardingValues = HashMultimap.create();
private boolean databaseShardingOnly;
private boolean masterRouteOnly;
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-657937431
Hi @SirMin sorry, do you mean the following one?
```
public final class HintManager implements AutoCloseable {
private static final ThreadLocal<HintManager> HINT_MANAGER_HOLDER = new ThreadLocal<>();
private final Multimap<String, Comparable<?>> databaseShardingValues = HashMultimap.create();
private final Multimap<String, Comparable<?>> tableShardingValues = HashMultimap.create();
private boolean databaseShardingOnly;
private boolean masterRouteOnly;
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-657937431
Hi @SirMin sorry, do you mean the following one?
```java
public final class HintManager implements AutoCloseable {
private static final ThreadLocal<HintManager> HINT_MANAGER_HOLDER = new ThreadLocal<>();
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kimmking commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
kimmking commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-658009949
You should implement an HintShardingAlgorithm to wrap Hint process.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-658042489
Hi @SirMin Thanks for your feedback.
I got your point. Maybe a new PR is needed to meet your needs.
IMO, I think your requirement is sensible in some way. It is preferable to return the original one from `HINT_MANAGER_HOLDER` rather than `new HintManager()` when call `getInstance()`. Besides, `clear()` is also expected to be called by the user themselves.
```java
/**
* Get a new instance for {@code HintManager}.
*
* @return {@code HintManager} instance
*/
public static HintManager getInstance() {
Preconditions.checkState(null == HINT_MANAGER_HOLDER.get(), "Hint has previous value, please clear first.");
HintManager result = new HintManager();
HINT_MANAGER_HOLDER.set(result);
return result;
}
```
**BTW, do you want to have a try to fix this issue by yourself? If not, we will get volunteers involved in it.**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] SirMin commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
SirMin commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-658032677
I know this, but I must save the HimManager instance and transfer it to the callee. Because setMasterRouteOnly and setDatabaseShardingValue call are not in the same place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] SirMin commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
SirMin commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-658050498
Ok. I'ill try to new pr
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-658042489
Hi @SirMin Thanks for your feedback.
I got your point. Maybe a new PR is needed to meet your needs.
IMO, I think your requirement is sensible in some way. It is preferable to return the original one from `HINT_MANAGER_HOLDER` rather than `new HintManager()` when call `getInstance()`. Besides, `clear()` is also expected to be called by the user themselves.
```java
/**
* Get a new instance for {@code HintManager}.
*
* @return {@code HintManager} instance
*/
public static HintManager getInstance() {
Preconditions.checkState(null == HINT_MANAGER_HOLDER.get(), "Hint has previous value, please clear first.");
// TODO You should pay more attention here.
HintManager result = new HintManager();
HINT_MANAGER_HOLDER.set(result);
return result;
}
```
**BTW, do you want to have a try to fix this issue by yourself? If not, we will get volunteers involved in it.**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-657937431
Hi @SirMin sorry, do you mean the following one? (Referring to our master branch)
```java
public final class HintManager implements AutoCloseable {
private static final ThreadLocal<HintManager> HINT_MANAGER_HOLDER = new ThreadLocal<>();
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] SirMin commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
SirMin commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-657987672
```
public static HintManager getInstance() {
Preconditions.checkState(null == HINT_MANAGER_HOLDER.get(), "Hint has previous value, please clear first.");
HintManager result = new HintManager();
HINT_MANAGER_HOLDER.set(result);
return result;
}
```
I want to set masterRouteOnly in Custom annotation and setDatabaseShardingValue in business code, But HintManager's getInstance return new everytime.
So i want to know why this is done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kimmking commented on issue #6342: Why does HintManager's getInstance return a new HintManager instead of returning the one held by the current thread
Posted by GitBox <gi...@apache.org>.
kimmking commented on issue #6342:
URL: https://github.com/apache/shardingsphere/issues/6342#issuecomment-658610604
@SirMin Assigned
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org