You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by yew1eb <gi...@git.apache.org> on 2017/10/24 17:05:31 UTC

[GitHub] flink pull request #4898: [FLINK-7706] [flip6] Add JobAccumulatorsHandler fo...

GitHub user yew1eb opened a pull request:

    https://github.com/apache/flink/pull/4898

    [FLINK-7706] [flip6] Add JobAccumulatorsHandler for new REST endpoint

    ## What is the purpose of the change
    
    *Port JobAccumulatorsHandler to REST endpoint*
    
    ## Brief change log
    
     - *Add JobAccumulatorsInfo class to describe the JSON format response*
     - *Add JobAccumulatorsHandler to deal with back pressure in rest server*
    
    ## Verifying this change
    
    This change added tests and can be verified as follows:
    
     - *JSON marshalling is tested via JobAccumulatorsInfoTest.*
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (yes / *no*)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / *no*)
      - The serializers: (yes / *no* / don't know)
      - The runtime per-record code paths (performance sensitive): (yes / *no* / don't know)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / *no* / don't know)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (yes / *no*)
      - If yes, how is the feature documented? (*not applicable* / docs / JavaDocs / not documented)
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/yew1eb/flink FLINK-7706

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/4898.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4898
    
----

----


---

[GitHub] flink pull request #4898: [FLINK-7706] [flip6] Add JobAccumulatorsHandler fo...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4898#discussion_r148802300
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/JobAccumulatorsInfo.java ---
    @@ -0,0 +1,128 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.rest.messages;
    +
    +import org.apache.flink.runtime.rest.handler.job.JobAccumulatorsHandler;
    +import org.apache.flink.util.Preconditions;
    +
    +import com.fasterxml.jackson.annotation.JsonCreator;
    +import com.fasterxml.jackson.annotation.JsonProperty;
    +
    +import javax.annotation.Nullable;
    +
    +import java.util.List;
    +import java.util.Objects;
    +
    +/**
    + * Response type of the {@link JobAccumulatorsHandler}.
    + */
    +public class JobAccumulatorsInfo implements ResponseBody {
    +	public static final String FIELD_NAME_JOB_ACCUMULATORS = "job-accumulators";
    +	public static final String FIELD_NAME_USER_TASK_ACCUMULATORS = "user-task-accumulators";
    +
    +	@JsonProperty(FIELD_NAME_JOB_ACCUMULATORS)
    +	private List<JobAccumulator> jobAccumulators;
    +
    +	@JsonProperty(FIELD_NAME_USER_TASK_ACCUMULATORS)
    +	private List<UserTaskAccumulator> userAccumulators;
    +
    +	@JsonCreator
    +	public JobAccumulatorsInfo(
    +		@JsonProperty(FIELD_NAME_JOB_ACCUMULATORS) @Nullable List<JobAccumulator> jobAccumulators,
    --- End diff --
    
    I would remove the `Nullable` and instead pass in an `Collections.emptyList()` if there are no job accumulators at the moment.


---

[GitHub] flink pull request #4898: [FLINK-7706] [flip6] Add JobAccumulatorsHandler fo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/4898


---

[GitHub] flink issue #4898: [FLINK-7706] [flip6] Add JobAccumulatorsHandler for new R...

Posted by yew1eb <gi...@git.apache.org>.
Github user yew1eb commented on the issue:

    https://github.com/apache/flink/pull/4898
  
    @tillrohrmann,  could you please take a look? :)


---

[GitHub] flink pull request #4898: [FLINK-7706] [flip6] Add JobAccumulatorsHandler fo...

Posted by yew1eb <gi...@git.apache.org>.
Github user yew1eb commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4898#discussion_r148934171
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/JobAccumulatorsInfo.java ---
    @@ -0,0 +1,128 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.rest.messages;
    +
    +import org.apache.flink.runtime.rest.handler.job.JobAccumulatorsHandler;
    +import org.apache.flink.util.Preconditions;
    +
    +import com.fasterxml.jackson.annotation.JsonCreator;
    +import com.fasterxml.jackson.annotation.JsonProperty;
    +
    +import javax.annotation.Nullable;
    +
    +import java.util.List;
    +import java.util.Objects;
    +
    +/**
    + * Response type of the {@link JobAccumulatorsHandler}.
    + */
    +public class JobAccumulatorsInfo implements ResponseBody {
    +	public static final String FIELD_NAME_JOB_ACCUMULATORS = "job-accumulators";
    +	public static final String FIELD_NAME_USER_TASK_ACCUMULATORS = "user-task-accumulators";
    +
    +	@JsonProperty(FIELD_NAME_JOB_ACCUMULATORS)
    +	private List<JobAccumulator> jobAccumulators;
    +
    +	@JsonProperty(FIELD_NAME_USER_TASK_ACCUMULATORS)
    +	private List<UserTaskAccumulator> userAccumulators;
    +
    +	@JsonCreator
    +	public JobAccumulatorsInfo(
    +		@JsonProperty(FIELD_NAME_JOB_ACCUMULATORS) @Nullable List<JobAccumulator> jobAccumulators,
    --- End diff --
    
    make sense


---