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/11/13 07:57:21 UTC
[GitHub] [shardingsphere-elasticjob] johnny2002 opened a new pull request #1721: #1720 Let user can customize the jobInstanceId
johnny2002 opened a new pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721
Fixes #1720 .
Changes proposed in this pull request:
Detect System Property "jobInstanceId", if it is exists, use IP concat with this property instead of jvm name.
-
-
----------------------------------------------------------------
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-elasticjob] TeslaCN commented on a change in pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#discussion_r522761955
##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/sharding/JobInstance.java
##########
@@ -34,10 +34,16 @@
private static final String DELIMITER = "@-@";
+ private static final String PARAM_JOB_INST_ID = "jobInstanceId";
+
private final String jobInstanceId;
public JobInstance() {
- jobInstanceId = IpUtils.getIp() + DELIMITER + ManagementFactory.getRuntimeMXBean().getName().split("@")[0];
+ String jobInstIdSuffix = System.getProperty(PARAM_JOB_INST_ID);
+ if (jobInstIdSuffix == null) {
Review comment:
Constant is on the left.
```suggestion
if (null == jobInstIdSuffix) {
```
----------------------------------------------------------------
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-elasticjob] johnny2002 commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727544478
> > > > > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
> > > >
> > > >
> > > > if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> > > > we need to customize the JobInstance class, while noway found.
> > >
> > >
> > > Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
> >
> >
> > but I need to know the JobInstance ID in sharing strategy implementation. I cannot find out a way, can you?
>
> May I know some details about your implementation of JobShardingStrategy? Or have you tried implementing your own JobItemExecutor?
my sharding strategy is that partition 1 assign to JobInstance1, partition 2 assign to JobInstance 2, and so on. I will show you the sample code later
----------------------------------------------------------------
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-elasticjob] codecov-io edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (137cd1b) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **increase** coverage by `0.00%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
=========================================
Coverage 86.06% 86.06%
Complexity 106 106
=========================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
=========================================
+ Hits 4890 4893 +3
Misses 477 477
Partials 315 315
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
| [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `82.53% <0.00%> (+1.58%)` | `1.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...981b14b](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] TeslaCN commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726636296
Please fix the checkstyle.
----------------------------------------------------------------
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-elasticjob] johnny2002 edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727530715
> The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
we need to customize the JobInstance class, while noway found. the best way is to provide a mechanism so that developers can extend the JobInstance class
----------------------------------------------------------------
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-elasticjob] johnny2002 edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727533125
> > > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
> >
> >
> > if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> > we need to customize the JobInstance class, while noway found.
>
> Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
as a matter of fact, user can see the job instance id in the console. and most of all, user can only distinguish JobInstances through id. in this way. you can't say it is an internal feature
----------------------------------------------------------------
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-elasticjob] johnny2002 commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726746549
BTW, changed constances to public, so that we can cut out the ID suffix, e.g. 10.0.0.110@-@001 to cut out "001"
----------------------------------------------------------------
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-elasticjob] johnny2002 commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727530715
> The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
we need to customize the JobInstance class, while noway found.
----------------------------------------------------------------
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-elasticjob] Technoboy- commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727336344
Hi, we discuss this pr and find that it may be no need to change it. We think the original way is enough and simple.
----------------------------------------------------------------
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-elasticjob] codecov-io edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (981b14b) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **decrease** coverage by `0.09%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
============================================
- Coverage 86.06% 85.96% -0.10%
Complexity 106 106
============================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
============================================
- Hits 4890 4887 -3
- Misses 477 482 +5
- Partials 315 316 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
| [...e/shardingsphere/elasticjob/infra/env/IpUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9lbnYvSXBVdGlscy5qYXZh) | `60.00% <0.00%> (-4.62%)` | `0.00% <0.00%> (ø%)` | |
| [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `77.77% <0.00%> (-3.18%)` | `1.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...7ac95c3](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] codecov-io edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (77f73c8) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **decrease** coverage by `0.01%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
============================================
- Coverage 86.06% 86.05% -0.02%
Complexity 106 106
============================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
============================================
+ Hits 4890 4892 +2
Misses 477 477
- Partials 315 316 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...77f73c8](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] johnny2002 commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727533125
> > > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
> >
> >
> > if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> > we need to customize the JobInstance class, while noway found.
>
> Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
as a matter of fact, user can see the job instance id in the console. and most of all, user can only distinguish JobInstances through id
----------------------------------------------------------------
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-elasticjob] johnny2002 commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727561497
samlple code:
```java
/**
* @author WeiZhou
*
* shardingTotalCount should be as same as JobInstances count.
* Neighbor 2 JobInstances backup eachother.
* Each partition will be assigned to same Jobinstance number or its backup.
* shardingTotalCount和jobInstances数量要一样.
* 相邻两个JobInstances互相备份.
* 每个作业分片要分配给相同号码的JobInstance或是它的备份单元
*/
@Slf4j
public class UnitizedAllocationJobShardingStrategy implements JobShardingStrategy {
int[] backupMap = new int[] { 2, 1, 4, 3, 6, 5, 8, 7 };//Unit 1<>2, 3<>4, 5<>6, 7<>8 backup eachother
@Override
public Map<JobInstance, List<Integer>> sharding(final List<JobInstance> jobInstances, final String jobName, final int shardingTotalCount) {
if (jobInstances.isEmpty()) {
return Collections.emptyMap();
}
JobInstance[] instArray = new JobInstance[shardingTotalCount];
for (JobInstance jobInst : jobInstances) {
//Job instance IDs like xxx@-@001, xxx@-@002, xxx@-@003 ...
String jobInstanceId = jobInst.getJobInstanceId();
jobInstanceId = jobInstanceId.substring(jobInstanceId.indexOf(JobInstance.DELIMITER) + JobInstance.DELIMITER.length());
int jobInstNum = Integer.parseInt(jobInstanceId);
instArray[jobInstNum - 1] = jobInst;
}
Map<JobInstance, List<Integer>> result = new LinkedHashMap<>(jobInstances.size(), 1);
for (int i = 0; i < shardingTotalCount; i++) {
//looup same number JobInstance
JobInstance locatedJobInst = instArray[i];
log.debug("master of {} is {}", i, locatedJobInst);
if (locatedJobInst == null) {
//lookup backup JobInstance
if (backupMap[i] - 1 >= instArray.length || instArray[backupMap[i] - 1] == null) {
throw new RuntimeException("No main or backup job executor found for unit:" + (i + 1) + ", backup unit:" + backupMap[i]);
}
locatedJobInst = instArray[backupMap[i] - 1];
log.debug("backup of {} is {}", i, locatedJobInst);
}
List<Integer> parts = result.get(locatedJobInst);
if (parts == null) {
parts = new ArrayList<>(2);
result.put(locatedJobInst, parts);
}
parts.add(i);
}
return result;
}
@Override
public String getType() {
return "UNIT_ALLOCATION";
}
}
```
----------------------------------------------------------------
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-elasticjob] johnny2002 edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727533125
> > > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
> >
> >
> > if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> > we need to customize the JobInstance class, while noway found.
>
> Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
as a matter of fact, user can see the job instance id in the console. and most of all, user can only distinguish JobInstances through id. in this way, you can't say it is an internal feature
----------------------------------------------------------------
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-elasticjob] TeslaCN commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727537190
> > > > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
> > >
> > >
> > > if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> > > we need to customize the JobInstance class, while noway found.
> >
> >
> > Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
>
> but I need to know the JobInstance ID in sharing strategy implementation. I cannot find out a way, can you?
May I know some details about your implementation of JobShardingStrategy? Or have you tried implementing your own JobItemExecutor?
----------------------------------------------------------------
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-elasticjob] codecov-io commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (4cdd47c) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **decrease** coverage by `0.09%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
============================================
- Coverage 86.06% 85.96% -0.10%
Complexity 106 106
============================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
============================================
- Hits 4890 4887 -3
- Misses 477 481 +4
- Partials 315 317 +2
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
| [...e/shardingsphere/elasticjob/infra/env/IpUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9lbnYvSXBVdGlscy5qYXZh) | `60.00% <0.00%> (-4.62%)` | `0.00% <0.00%> (ø%)` | |
| [...asticjob/lite/internal/storage/JobNodeStorage.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc3RvcmFnZS9Kb2JOb2RlU3RvcmFnZS5qYXZh) | `87.50% <0.00%> (-3.58%)` | `0.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...4cdd47c](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] johnny2002 commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
johnny2002 commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727532629
> > > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
> >
> >
> > if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> > we need to customize the JobInstance class, while noway found.
>
> Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
but I need to know the JobInstance ID in sharing strategy implementation. I cannot find out a way, can you?
----------------------------------------------------------------
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-elasticjob] codecov-io edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (981b14b) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **increase** coverage by `0.00%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
=========================================
Coverage 86.06% 86.06%
Complexity 106 106
=========================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
=========================================
+ Hits 4890 4893 +3
Misses 477 477
Partials 315 315
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
| [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `82.53% <0.00%> (+1.58%)` | `1.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...7ac95c3](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] TeslaCN commented on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-727532046
> > The proposal of this pr still need to be discussed, I post my comment in the issue, we can talk about it there.
>
> if we don't make this changes, then there is no way to fullfill our requirement. our requirement is that we want to assign specific partition to specific job instance.
> we need to customize the JobInstance class, while noway found.
Sorry to take up your time. I realized that the jobInstanceId is an internal implementation of ElasticJob and it is not recommended to expose to user. Could you consider defining your own instanceId in somewhere?
----------------------------------------------------------------
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-elasticjob] codecov-io edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (77f73c8) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **decrease** coverage by `0.06%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
============================================
- Coverage 86.06% 85.99% -0.07%
Complexity 106 106
============================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
============================================
- Hits 4890 4889 -1
- Misses 477 478 +1
- Partials 315 318 +3
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
| [...e/shardingsphere/elasticjob/infra/env/IpUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9lbnYvSXBVdGlscy5qYXZh) | `60.00% <0.00%> (-4.62%)` | `0.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...77f73c8](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] codecov-io edited a comment on pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#issuecomment-726590935
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=h1) Report
> Merging [#1721](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=desc) (137cd1b) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/aff2df57b49c0c19b9cd2f015f86206f0fb055da?el=desc) (aff2df5) will **decrease** coverage by `0.11%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1721 +/- ##
============================================
- Coverage 86.06% 85.94% -0.12%
Complexity 106 106
============================================
Files 252 252
Lines 5682 5685 +3
Branches 878 879 +1
============================================
- Hits 4890 4886 -4
- Misses 477 482 +5
- Partials 315 317 +2
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...elasticjob/infra/handler/sharding/JobInstance.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYkluc3RhbmNlLmphdmE=) | `85.71% <75.00%> (-14.29%)` | `0.00 <0.00> (ø)` | |
| [...e/shardingsphere/elasticjob/infra/env/IpUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9lbnYvSXBVdGlscy5qYXZh) | `60.00% <0.00%> (-4.62%)` | `0.00% <0.00%> (ø%)` | |
| [...ite/internal/election/ElectionListenerManager.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvZWxlY3Rpb24vRWxlY3Rpb25MaXN0ZW5lck1hbmFnZXIuamF2YQ==) | `92.00% <0.00%> (-4.00%)` | `0.00% <0.00%> (ø%)` | |
| [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `77.77% <0.00%> (-3.18%)` | `1.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=footer). Last update [aff2df5...981b14b](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1721?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-elasticjob] TeslaCN commented on a change in pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#discussion_r522761700
##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/sharding/JobInstance.java
##########
@@ -34,10 +34,16 @@
private static final String DELIMITER = "@-@";
+ private static final String PARAM_JOB_INST_ID = "jobInstanceId";
Review comment:
Name it `PROPERTY_JOB_INSTANCE_ID` may be better.
##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/sharding/JobInstance.java
##########
@@ -34,10 +34,16 @@
private static final String DELIMITER = "@-@";
+ private static final String PARAM_JOB_INST_ID = "jobInstanceId";
+
private final String jobInstanceId;
public JobInstance() {
- jobInstanceId = IpUtils.getIp() + DELIMITER + ManagementFactory.getRuntimeMXBean().getName().split("@")[0];
+ String jobInstIdSuffix = System.getProperty(PARAM_JOB_INST_ID);
+ if (jobInstIdSuffix == null) {
Review comment:
Constant is on the left.
----------------------------------------------------------------
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-elasticjob] TeslaCN commented on a change in pull request #1721: #1720 Let user can customize the jobInstanceId
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1721:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1721#discussion_r522806944
##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/sharding/JobInstance.java
##########
@@ -34,10 +34,16 @@
private static final String DELIMITER = "@-@";
+ private static final String PROPERTY_JOB_INSTANCE_ID = "jobInstanceId";
+
private final String jobInstanceId;
public JobInstance() {
- jobInstanceId = IpUtils.getIp() + DELIMITER + ManagementFactory.getRuntimeMXBean().getName().split("@")[0];
+ String jobInstIdSuffix = System.getProperty(PROPERTY_JOB_INSTANCE_ID);
Review comment:
It's better to keep the names consistent.
```suggestion
String jobInstanceIdSuffix = System.getProperty(PROPERTY_JOB_INSTANCE_ID);
```
----------------------------------------------------------------
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